-
Notifications
You must be signed in to change notification settings - Fork 542
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
Adjust referrers fallback behavior to be optional #1748
base: main
Are you sure you want to change the base?
Conversation
I was talking with Jon about using `crane cp` in a use case where I never want it to accidentally surprise me with the fallback tags, so this implements a very basic means of making that behavior optional.
Delicious tests pointing out that this is a breaking change as-is because it's opt-in instead of opt-out, as suggested by Jon -- I'm happy to adjust tests or behavior as desired, but y'all maintainers probably need to decide which direction you'd like to go and give me some suggestion (or send me away). 🙇 😂 |
This Pull Request is stale because it has been open for 90 days with |
This Pull Request is stale because it has been open for 90 days with |
Begone, ye stale bot |
@@ -81,6 +82,9 @@ func (f *fetcher) fetchReferrers(ctx context.Context, filter map[string]string, | |||
} else if err != nil { | |||
return nil, err | |||
} | |||
} else { | |||
// TODO just returning "empty" here doesn't feel quite right -- maybe transport.CheckError again but this time letting http.StatusBadRequest return an error? | |||
return empty.Index, nil |
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.
Coming back to this almost a year later and feeling like a nice fmt.Errorf
is probably more appropriate here. Thoughts? Maybe the variable needs to be opt-out instead of opt-in, also? (GGCR_REF_NO_FALLBACK
or GGCR_REF_NEVER_FALLBACK
?)
38b69ff if you wanna see where I finally got cheeky and have applied my own patch to my personal builds 😄 |
This Pull Request is stale because it has been open for 90 days with |
This Pull Request is stale because it has been open for 90 days with |
I was talking with @jonjohnsonjr about using
crane cp
in a use case where I never want it to accidentally surprise me with the fallback tags, so this implements a very basic means of making that behavior optional.I figured having a PR would give us a stronger straw-man to discuss the finer details around, especially since I don't feel like I know this library/codebase well enough to make actual informed recommendations for how I'd like to see this implemented. 😅
(As always, happy to rebase, amend, adjust, rewrite, reword, close, let someone else take over, etc as desired!)