Skip to content

Conversation

@vrothberg
Copy link
Member

It is a common pitfall to add a trailing slash when specifying a
registry (e.g., "docker.io/library/" version "docker.io/library").
Remove trailing slashes to normalize the representation of registries.

Signed-off-by: Valentin Rothberg vrothberg@suse.com

@TomSweeneyRedHat
Copy link
Member

@vrothberg looks like you need a merge

@TomSweeneyRedHat
Copy link
Member

@baude PTAL

@baude
Copy link
Member

baude commented Mar 15, 2018

@vrothberg I think you need to also make a code change at https://github.com/projectatomic/registries as well.

@baude
Copy link
Member

baude commented Mar 15, 2018

else LGTM

@vrothberg
Copy link
Member Author

@baude I will look into https://github.com/projectatomic/registries 👍

@vrothberg
Copy link
Member Author

@TomSweeneyRedHat @baude I don't see why this change could cause the Travis fail:

cmd/skopeo/inspect.go:100:11: cannot use imgInspect.Created (type *time.Time) as type time.Time in field value

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 15, 2018

I don't see why this change could cause the Travis fail:

cmd/skopeo/inspect.go:100:11: cannot use imgInspect.Created (type *time.Time) as type time.Time in field value

@vrothberg That’s not a fault of this change, you happened to submit a PR between merging #416 and containers/skopeo#477 .

This should disappear after triggering the tests again.

@vrothberg
Copy link
Member Author

Thanks @mtrmac, it really got me confused 😆

Copy link
Collaborator

@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.

LGTM pending tests (which I have just poked)

@rhatdan
Copy link
Member

rhatdan commented Mar 15, 2018

@runcom PTAL

@rhatdan
Copy link
Member

rhatdan commented Mar 16, 2018

@mtrmac Tests pass.

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 16, 2018

LGTM again for pullaprove. Up to @runcom now.

Approved with PullApprove

@runcom
Copy link
Member

runcom commented Mar 16, 2018

LGTM

Approved with PullApprove

@runcom
Copy link
Member

runcom commented Mar 16, 2018

needs one last rebase and I'll merge

It is a common pitfall to add a trailing slash when specifying a
registry (e.g., "docker.io/library/" version "docker.io/library").
Remove trailing slashes to normalize the representation of registries.

Signed-off-by: Valentin Rothberg <vrothberg@suse.com>
@vrothberg
Copy link
Member Author

@runcom I've rebased the branch. Thanks to all for the feedback and the reviews.

Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

LGTM

@runcom
Copy link
Member

runcom commented Mar 16, 2018

@runcom I've rebased the branch. Thanks to all for the feedback and the reviews.

You're super welcome, thanks for this PR :)

@mtrmac mtrmac merged commit 2d0fc4b into containers:master Mar 16, 2018
@vrothberg vrothberg deleted the trim-slashes branch March 17, 2018 10:23
rh-atomic-bot pushed a commit to projectatomic/registries that referenced this pull request Apr 2, 2018
This is a follow-up change for containers/image#428.

It is a common pitfall to add a trailing slash when specifying a
registry (e.g., "docker.io/library/" version "docker.io/library").
Remove trailing slashes to normalize the representation of registries.

To facilitate testing, don't overwrite the BINARY and MIGRATOR variable
when being set in test.bats.

Signed-off-by: Valentin Rothberg <vrothberg@suse.com>

Closes: #32
Approved by: baude
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.

6 participants