Skip to content
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

Support version label, drop legacy stuff, print information before pulling #22

Merged
merged 2 commits into from
Nov 2, 2018

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Nov 1, 2018

See individual commit messages.

Check for the standard `version` label instead of the old
`io.openshift.os-version`. Also just completely drop support for
`io.openshift.os-commit`. It's still early and we're moving fast so I
think it's not worth it yet to burden ourselves with legacy stuff.

Also fix minor typo in error message.
That way we know what we're pulling, before pulling it.
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 1, 2018
@jlebon
Copy link
Member Author

jlebon commented Nov 1, 2018

Noticed this while testing openshift/machine-config-operator#151.

@@ -97,17 +97,14 @@ func Execute(cmd *cobra.Command, args []string) {
}

// Make sure it has a commit label before pulling
ostree_csum, ok := imagedata.Labels["io.openshift.os-commit"]
ostree_csum, ok := imagedata.Labels["com.coreos.ostree-commit"]
Copy link
Member

Choose a reason for hiding this comment

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

Replacing io.openshift.os-commit with com.coreos.ostree-commit? Just verifying as the commit message notes dropping support rather than switching.

Copy link
Member Author

Choose a reason for hiding this comment

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

com.coreos.ostree-commit was already supported as a fallback, this is just dropping support for the old one.

@ashcrow
Copy link
Member

ashcrow commented Nov 2, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 2, 2018
@ashcrow ashcrow merged commit eb6cc16 into openshift:master Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants