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

Add --platform CLI option for crane #788

Merged
merged 3 commits into from
Oct 22, 2020

Conversation

dvob
Copy link
Contributor

@dvob dvob commented Oct 20, 2020

Adds a --platform option for the crane CLI tool.

I added a if-statement in pkg/crane/options.go that we don't get a nil pointer dereference if WithPlatform is called with nil. We could also catch this in cmd/crane/cmd/root.go` but I think it would in general make sense to avoid the nil pointer dereference.

I changed the error output in pkg/v1/remote/index.go to the form os/arch. This is not necessary but i think we should use the same form in the error output and the flag. I think this is quite arbitrary but I prefer os/arch over arch/os.

platform *v1.Platform
}

func (pv *platformValue) Set(platform string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll probably also want to allow a third value for the variant. We do this in ko and I'd like to be consistent.

containerd also allows this, and in the fullness of time I'd like to be compatible with what they allow, but for now let's just do the os/arch/variant form.

@jonjohnsonjr
Copy link
Collaborator

This is not necessary but i think we should use the same form in the error output and the flag. I think this is quite arbitrary but I prefer os/arch over arch/os.

I think that's fine, we use that same order in ko, so that works for me: https://github.com/google/ko#multi-platform-images

@dvob
Copy link
Contributor Author

dvob commented Oct 21, 2020

Added support for variant.

Copy link
Collaborator

@jonjohnsonjr jonjohnsonjr left a comment

Choose a reason for hiding this comment

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

Thanks so much!

@jonjohnsonjr
Copy link
Collaborator

Ah the travis failures -- you'll need to run ./hack/update-codegen.sh to regenerate the docs.

@jonjohnsonjr jonjohnsonjr merged commit 25dfb32 into google:master Oct 22, 2020
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.

None yet

2 participants