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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move image pull args into struct #2891
base: main
Are you sure you want to change the base?
Conversation
d055293
to
174713e
Compare
77e11ab
to
cb5159c
Compare
Rebased with main and addressed reviews. Wondering why lint check seems to fail though... |
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, one final comment. Thanks.
cb5159c
to
c7951ed
Compare
Can I just confirm that the lint check failing is unrelated? |
I think it's unrelated. You can rebase which should retrigger the CI. |
c7951ed
to
02310d8
Compare
Rebased Rebase below this fixes lint issues |
Signed-off-by: David Son <davbson@amazon.com>
02310d8
to
6dd2f6c
Compare
In the image pull workflow, the function headers are getting far too large. (imgutil.EnsureImage had 13 required arguments as of the time of this PR.) This change consolidates many of the arguments in
imgutil.PullImage
intotypes.ImagePullTypes
, and the necessary refactors that come with this change.I did as much pre-processing in
processPullCommandFlags
as possible. So, anything that cannot be declared from the function flags did not fall in the options.Did some basic testing with
make
and running some basic image commands to do a sanity check that functionality remained intact.Very open to suggestions on how to structure this a bit more logically. I made a best effort but a fresh pair of eyes always helps 馃槄