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

Use zip archives for phylum update #225

Merged
merged 7 commits into from
Apr 6, 2022
Merged

Conversation

kylewillmon
Copy link
Contributor

@kylewillmon kylewillmon commented Apr 5, 2022

Overview

Implement phylum update by downloading the appropriate zip file and running install.sh

Checklist

  • Does this PR have an associated issue?
  • Have you ensured that you have met the expected acceptance criteria?
  • Have you created sufficient tests?
  • Have you updated all affected documentation?

@kylewillmon kylewillmon marked this pull request as ready for review April 5, 2022 21:05
@kylewillmon kylewillmon requested a review from a team April 5, 2022 21:05
@kylewillmon kylewillmon requested a review from a team as a code owner April 5, 2022 21:05
maxrake
maxrake previously approved these changes Apr 6, 2022
Copy link
Contributor

@maxrake maxrake left a comment

Choose a reason for hiding this comment

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

I reviewed all the files. I don't know Rust well enough to claim that those files are good...but they appear to be good.

Trying the build artifacts resulted in no big issues found. I couldn’t try updating to the latest non-prerelease b/c that is still 1.2.0, which has the old artifact name/scheme...and the failure experienced there is expected. Updating to a prerelease version worked.

It would be nice to have others who are more familiar with Rust complete the review...but I'm providing my approval now based on the fact that the changes are confirmed to do what they say, based on manual verification testing.

Copy link
Contributor

@cd-work cd-work left a comment

Choose a reason for hiding this comment

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

I haven't run it myself, but these are some Rust style issues I've found.

Most of them are based on style guidelines from other projects, so please let me know if they conflict with any style guidelines in place already.

cli/src/update/unix.rs Outdated Show resolved Hide resolved
cli/src/update/unix.rs Outdated Show resolved Hide resolved
cli/src/update/unix.rs Outdated Show resolved Hide resolved
cli/src/update/unix.rs Outdated Show resolved Hide resolved
cli/src/update/unix.rs Outdated Show resolved Hide resolved
cli/src/update/unix.rs Outdated Show resolved Hide resolved
cli/src/update/unix.rs Outdated Show resolved Hide resolved
cli/src/update/unix.rs Show resolved Hide resolved
@kylewillmon kylewillmon requested a review from cd-work April 6, 2022 15:26
@kylewillmon kylewillmon merged commit ad23cdb into development Apr 6, 2022
@kylewillmon kylewillmon deleted the update-refactor branch April 6, 2022 15:55
@kylewillmon kylewillmon linked an issue Apr 6, 2022 that may be closed by this pull request
eeclfrei pushed a commit that referenced this pull request Apr 10, 2022
* Use zip file in `phylum update`
* Allow `phylum update` without authentication
* Explain update from 1.2.0 in docs

Co-authored-by: Christian Dürr <[email protected]>
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.

phylum update subcommand is broken
3 participants