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

Allow mocking of url downloads with local file (--archive) #67

Merged
merged 1 commit into from Oct 11, 2018

Conversation

ahmetb
Copy link
Member

@ahmetb ahmetb commented Oct 7, 2018

  • rename --source to --manifest in install command.
  • introduce --archive flag to install command to override the installation from
    the uri specified in the manifest file.
  • remove getFileFromArg() method (not needed)

This patch allows specifying both plugin yamls and archives to install cmd:

krew install --manifest foo.yaml --archive foo.zip

It does not download a file, but it still checks integrity (so it can be used to
check the manifest without uploading the archive anywhere).

The method signatures got very long but I think that's partially because
we probably didn't design the methods in install.go well (the code smell here
is the repetitive args passed from one method to another all the way down).

@codecov-io
Copy link

codecov-io commented Oct 7, 2018

Codecov Report

Merging #67 into master will decrease coverage by 0.47%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
- Coverage    47.8%   47.32%   -0.48%     
==========================================
  Files          12       12              
  Lines         592      598       +6     
==========================================
  Hits          283      283              
- Misses        262      268       +6     
  Partials       47       47
Impacted Files Coverage Δ
pkg/installation/install.go 31.18% <0%> (-1.04%) ⬇️
pkg/download/fetch.go 0% <0%> (ø) ⬆️
pkg/installation/upgrade.go 0% <0%> (ø) ⬆️

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 94c644f...f498a4a. Read the comment docs.

@ahmetb
Copy link
Member Author

ahmetb commented Oct 9, 2018

@lbb wdyt of this

@ahmetb ahmetb changed the title Allow mocking of url downloads with local file Allow mocking of url downloads with local file (--archive) Oct 10, 2018
@ahmetb ahmetb force-pushed the mock-file branch 5 times, most recently from e9544ae to 000f7dc Compare October 10, 2018 22:08
@ahmetb
Copy link
Member Author

ahmetb commented Oct 10, 2018

Ready for review.

- rename --source to --manifest in install command.
- introduce --archive flag to install command to override the installation from
  the uri specified in the manifest file.
- remove getFileFromArg() method (not needed)

This patch allows specifying both plugin yamls and archives to install cmd:

    krew install --manifest foo.yaml --archive foo.zip

It does not download a file, but it still checks integrity (so it can be used to
check the manifest without uploading the archive anywhere).

The method signatures got very long but I think that's partially because
we probably didn't design the methods in install.go well (the code smell here
is the repetitive args passed from one method to another all the way down).

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
Copy link
Contributor

@lbb lbb left a comment

Choose a reason for hiding this comment

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

Looks good :)

cmd/krew/cmd/install.go Show resolved Hide resolved
@ahmetb ahmetb merged commit 7c62568 into kubernetes-sigs:master Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants