-
Notifications
You must be signed in to change notification settings - Fork 84
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
feat: add check updates command #577
base: master
Are you sure you want to change the base?
Conversation
b2f3436
to
dbe15b4
Compare
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.
Looking good and just a few comments.
Can you also please add descriptions to any PRs so it makes it easier to review. This should include things like
- Added commands (screenshots of the output is very useful)
- Changes to behaviour.
@@ -42,3 +42,22 @@ macro_rules! interact_or { | |||
} | |||
}; | |||
} | |||
|
|||
#[macro_export] | |||
macro_rules! check_update { |
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.
Why does this have to be a macro instead of just a function? In general I'd prefer to avoid macros unless absolutely necessary.
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.
It's a macro because I can overload the call signature of it to default the second parameter to false
, but I can make it into a function if you want.
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.
I think a function is preferred here for simplicity. If you want to avoid having to provided false
as the second parameter, we can have a check_update_command
function that passed false
simplified check_updates command fix: more user-friendly error message for check_updates on Config write failure
No description provided.