-
Notifications
You must be signed in to change notification settings - Fork 360
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
Improve the messages when a package is not up-to-date on opam upgrade #6272
Conversation
f624b0b
to
eca8fcd
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.
It's missing test update, otherwise, lgtm!
@@ -330,9 +333,11 @@ let upgrade_t | |||
) (OpamPackage.Set.elements unopt)) | |||
); | |||
OpamConsole.formatted_msg | |||
"However, you may \"opam upgrade\" these packages explicitly, \ | |||
"However, you may \"opam upgrade\" these packages explicitly \ | |||
at these versions (e.g. \"opam upgrade %s\"), \ |
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.
at these versions (e.g. \"opam upgrade %s\"), \ | |
at these versions (e.g. 'opam upgrade %s'), \ |
+ some underline ?
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.
the single quotes clashes with the double-quotes used prior in the same sentense. I think it's better to stay consistent
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 underline, i'm not sure if it's necessary. The rest of the opam example commands in our codebase uses either double-quotes like here or bold
, the former being used more often.
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.
They are both used in fact, "
and '
.
eca8fcd
to
e721c73
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.
Thanks!
Fixes #6270 and also the confusing message about
--verbose
when it is unnecessary.