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

Refactor [g]crane to make threading options easier #845

Merged
merged 6 commits into from
Nov 30, 2020

Conversation

jonjohnsonjr
Copy link
Collaborator

@jonjohnsonjr jonjohnsonjr commented Nov 26, 2020

I've split out some of this from #837

This addresses a few things:

  1. gcrane now inherits the options/flags from crane (e.g. --insecure, --platform, -v, the headerTransport thing). Some of these were duplicated code, others were just broken in gcrane.
  2. Drop most use of init for the cmd packages
  3. Allow passing in a keychain or auth to crane (Fixes Adds WithAuth functional option to pkg/crane to allow for alternate m… #653).
  4. Create and thread gcrane.Keychain through as the default keychain for gcrane (Fixes Use Google keychain on gcrane commands #721).

I ended up passing a pointer to a slice of options through the command init logic, which feels kind of gross, but a lot less gross than having package-level globals... let me know if you have a better suggestion. I think we need to do this because PersistentPreRun has to modify the options, but we've already created each command by the time that runs... :/

This will just overwrite certain portions of crane now so that flags and
options and stuff stay in sync.
@codecov-io
Copy link

codecov-io commented Nov 26, 2020

Codecov Report

Merging #845 (7d765ad) into master (93b58dc) will decrease coverage by 0.17%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #845      +/-   ##
==========================================
- Coverage   74.77%   74.59%   -0.18%     
==========================================
  Files         106      106              
  Lines        4420     4421       +1     
==========================================
- Hits         3305     3298       -7     
- Misses        626      633       +7     
- Partials      489      490       +1     
Impacted Files Coverage Δ
pkg/crane/options.go 52.94% <0.00%> (-16.29%) ⬇️
pkg/gcrane/copy.go 84.37% <75.00%> (-1.14%) ⬇️
pkg/v1/tarball/write.go 61.45% <0.00%> (-0.98%) ⬇️
pkg/v1/remote/image.go 79.45% <0.00%> (+0.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93b58dc...7d765ad. Read the comment docs.

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

3 participants