Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ashlar #3551
Ashlar #3551
Changes from 129 commits
92ef528
715a1c4
10f2054
91b8e3a
e09d8ad
84269e4
724412e
62e16c6
ccdc22d
b4178e7
11ee7b8
7e20a5d
c85d04d
2a8ea15
68e4189
6ad6999
54c9029
cb26c76
828aba3
63e7ebc
4970bd3
deb9bb5
c47d3b6
d555aee
c8ac4fb
31acde9
288f590
6843268
69a807c
eaa39c3
b57595a
125281f
9f98637
9c17dff
1f45791
9bc2269
f6bedd0
afb2c27
2329c99
307d803
2d5091c
bbfa613
edc4998
12ad5df
eea6c46
ba68c8e
9996fa7
c513dbb
71b4be6
aff97c0
356f478
c809ad4
14fb0dc
b111ed2
b3fbd52
a5165c6
5487b2c
95160c5
2d9411c
79a1ef4
2c133d6
0828127
46451f7
c908aea
5c3ce93
f1a99f2
b26f50a
f567546
77d4e0f
f693887
f50061d
b133416
3fb1383
fa2e25d
f8c84ca
efe10c8
854865b
9709eb9
5725872
8a7dae8
1bad2c1
a8e7105
fb3833c
14f1034
f73e168
cd4a506
2e3bd97
27a0c1d
19ec447
a2a5c96
352d579
ff0ef63
97c5beb
d73023b
c8bdccd
c1f36ae
9b018f2
cc5c668
1e03f60
fca179d
3a5e027
b746eea
00077da
7d8c759
e9d0b19
aa88e25
2be482d
06e9913
6e192d7
78baab7
a89bfaf
784057d
e660f93
6d7fa19
059c947
3b26a84
65f2e98
0e5e56b
8bec89d
50d4a6c
4521bb8
056a240
dbbfeb3
d05796a
57a8dc4
146534a
76ee81e
b92c085
440f140
10bf81e
d5d60ff
7fa6f95
7988e55
4aa7498
926caa7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's so ugly I love it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to just remove the UUID instead of replacing it with zeros? Does that break stuff?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea whether any downstream tool may care about it or not.
I considered just finding and replacing everything after the
urn:uuid
, but didn't want to accidentally match anything else.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SPPearce Can you comment on why you changed from
sed -i
(in-place) to operating on a copy? These images can be hundreds of GBs so we try to avoid making copies where possible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
-i
flag didn't seem to be working on the CI for some reason. I definitely agree that would be better.One thought is that the metadata is at the very end of the tif from what I could see, so there might be some clever sed (or similar) magic that could be used to start from the end of a file and only make one change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, the test.yml was broken in your commit that failed CI. The file tests were changed from
md5sum
tocontains
with the placeholder TODO message. That's since been fixed so we're going to try again withsed -i
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It now passes all tests.
Thank you for figuring that out @jmuhlich!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an acceptable change @SPPearce?