-
Notifications
You must be signed in to change notification settings - Fork 136
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
fuelup check
#64
fuelup check
#64
Conversation
Maybe add an option that explicitly lists bundled plugins, e.g. |
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.
For the most part looking good!
Added a couple comments for now, but might be a little easier to review after landing #73 and merging or rebasing onto it.
…gh/fuelup-check
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.
Closes #16
(note: this merges into #62)
Now that we have
DownloadCfg
in #62 it's much easier to supportfuelup check
, since all we need to do is to build the cfg and compare it with the current version running on the machine.Usage
There is definitely a more elegant way of handling checks - i.e. we shouldn't panic on each command but simply collect the results and print it all at the end. For example, if:fuel-core
was not installed,fuelup check
would panic, but ideally it should do something like thisHave since updated this PR by not panicking if the component cannot be found/run, instead showing a
not found
message. I was wondering if there should be an action message here i.e. if any offorc
,forc-fmt
,forc-explore
, orforc-lsp
are missing or outdated, should we add a reminder to dofuelup install
? Then again it might be intentionally not updated on purpose (if sayforc
has a breaking change that breaks a dev's project). Due to this reason I decided not to include this action message.Also included plugins in the check.