fix(cli/rustup-mode): improve exit code of rustup check#4694
fix(cli/rustup-mode): improve exit code of rustup check#4694rami3l merged 2 commits intorust-lang:mainfrom
rustup check#4694Conversation
|
How one is supposed to know which error code is returned when and what it corresponds to? The error code type is just a wrapped around an integer and it is not documented on the type itself either. |
|
@appleGun22 Please feel free to make a PR adding documentation to an appropriate place once this decision has been settled, preferably in the |
This change follows the convention of DNF where `0` is returned when no updates and `100` when updates are available.
rami3l
force-pushed
the
fix/check-exit-code
branch
from
634d8e8 to
9becf3b
Compare
There was a problem hiding this comment.
Hm, it doesn't look like there's a strong consensus from package managers around this but I'm happy to do something that seems reasonable and has some precedence.
I admit I'm slightly worried that this could break existing scripts though on balance I think this is worth doing. As noted on the issue, rustup check was intended for interactive use and isn't particularly amenable to scripting (hence the motivation for the change).
I do have one nit but I'm not inclined to block on it.
|
|
||
| let exit_status = if update_available { 0 } else { 1 }; | ||
| Ok(ExitCode(exit_status)) | ||
| Ok(ExitCode(if update_available { 100 } else { 0 })) |
There was a problem hiding this comment.
I do think we should start using constants for exit codes (e.g. EXIT_SUCCESS, EXIT_UPDATE_AVAILABLE, etc). If not now then soon. Especially if we want to make any more commands return special exit codes.
There was a problem hiding this comment.
@ChrisDenton Nice catch! I'd say that might belong to a different patch though.
@FranciscoTGouveia would you be interested in trying this one?
|
@ChrisDenton Thanks! The exit code differences are a new behavior in this release already so the breakages should be controlled. |
|
For sure. The behaviour change from current stable to current beta is that if you were relying on it not failing (e.g. in a |
|
Sorry for being slow to review -- LGTM! |
4 participants
Closes #4681.
Following the feedback on #4340 and particularly the previous discussion in #4681 (comment), this change follows the convention of DNF where
0is returned when no updates and100when updates are available.