RFC: Consolidate getting signatures #1417
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dd80531 to
cf6a370
Compare
vrothberg
reviewed
Nov 29, 2021
Member
vrothberg
left a comment
There was a problem hiding this comment.
Both look like an improvement to me.
LGTM
eded5a3 to
2d44e7b
Compare
2d44e7b to
371b442
Compare
371b442 to
adf86e3
Compare
adf86e3 to
312fb65
Compare
312fb65 to
fe13b0e
Compare
Member
|
LGTM |
This should not make a difference right now, because nothing else uses the UnparsedImage, but it allows caching _in principle_ and it makes the code more similar to the copyOneImage version, which we'll benefit from next. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
fe13b0e to
1bdda15
Compare
Collaborator
Author
|
I’m not that happy about the end state, but I don’t think I will think of a better approach with any more time, so let’s do that. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Consolidate figuring out which signatures from the source to copy, and whether that can work, into a helper.
@vrothberg RFC — I’m honestly rather unhappy about passing the progress messages into the helper (and, at the same time, unwilling to make the messages less descriptive just to share a few lines of code); basically just to show that something like this is possible, and solicit better ideas.
At this point I’d really prefer only the first commit to be merged.