-
Notifications
You must be signed in to change notification settings - Fork 339
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 for ECR Public #253
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for the quick turnaround, lgtm
@samuelkarp Any updates here, looks like the test failed for dependencies? |
@PatrickXYS Considering I've been on vacation since right after opening this PR, no. I don't currently have access to a Mac. It looks like an update to |
I'm inclined to drop Go 1.11 support in macOS and other OSes.
What do you think? Are there other places where customers have to use Go 1.11? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still reading through, flushing minor nits for now.
@kzys I'm comfortable with that proposal. I think the only thing that needs to be updated is the GitHub actions workflow as the readme already recommends 1.12+ (though possibly we could bump that up to 1.14+ as well). My preference would be to have that change come in as a separate PR and then rebase this one on top. |
This change removes Go 1.11 from GitHub Actions to unblock awslabs#253. Go 1.11 doesn't work on macOS 10.05 after upgrading golang.org/x/sys. While we could downgrade golang.org/x/sys, Go 1.11 is no longer supported by the Go team and most OSes tend to have newer Go packages. Signed-off-by: Kazuyoshi Kato <[email protected]>
2b500b9
to
952353a
Compare
952353a
to
e252fbf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Issue #, if available:
Fixes #248
Depends on #252
Description of changes:
This set of commits adds support for ECR Public. Note that it depends on #252, so you can skip those commits when reviewing this one (or they'll disappear from the diff when we merge that PR).
ECR Public credentials are cached and returned by the list command by default.
/cc @micahhausler
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.