Skip to content

Conversation

@runcom
Copy link
Collaborator

@runcom runcom commented Nov 8, 2016

No description provided.

@runcom runcom force-pushed the pull-by-digest-1.12.3 branch from 365e457 to 8a40a88 Compare November 8, 2016 14:55
@runcom runcom changed the title [DO NOT MERGE] [1.12.3] Signature verification: fix pull-by-digest [1.12.3] Signature verification: fix pull-by-digest Nov 8, 2016
@runcom
Copy link
Collaborator Author

runcom commented Nov 8, 2016

Finally good to go, @rhatdan @mtrmac

@rhatdan
Copy link
Member

rhatdan commented Nov 8, 2016

LGTM, but Lets get @mtrmac review.

Copy link

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more or less a vendoring of containers/image, right? If so then that should be fine.

There does seem to be some discrepancy between the comments above the tests and the actual tests AFIACT, though. I don’t have a terribly strong opinion on whether that should be resolved by adding tests or removing comments :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can’t see a test for pulling by tag.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the one above this?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, s/that I can pull by a digest and tag/thaat I can also pull by a digest/?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can’t see a test for pulling by tag.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the first one in this chain

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@runcom runcom force-pushed the pull-by-digest-1.12.3 branch from 8a40a88 to 3502b63 Compare November 8, 2016 15:26
@runcom
Copy link
Collaborator Author

runcom commented Nov 8, 2016

@mtrmac fixed

This is more or less a vendoring of containers/image, right? If so then that should be fine.

correct

@runcom
Copy link
Collaborator Author

runcom commented Nov 8, 2016

@rhatdan good to go now!

@rhatdan rhatdan merged commit 0d85489 into projectatomic:docker-1.12.3 Nov 8, 2016
@runcom runcom deleted the pull-by-digest-1.12.3 branch November 8, 2016 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants