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.

Sign up for GitHub

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

Jump to bottom

Revert " imp: Add exitcode::USAGE exit code as suggested in #1327" #1653

Merged
merged 2 commits into from
Feb 3, 2020
Merged

Revert " imp: Add exitcode::USAGE exit code as suggested in #1327" #1653

merged 2 commits into from
Feb 3, 2020

Conversation

Dylan-DPC-zz
Copy link

@Dylan-DPC-zz Dylan-DPC-zz commented Feb 1, 2020 •
edited by pksunkara

Reverts #1637

@CreepySkeleton
Copy link
Contributor

We should also document this exit codes somewhere. Maybe it worth looking at other UNIX apps?

pksunkara
pksunkara reviewed Feb 1, 2020
View reviewed changes
src/build/app/mod.rs Outdated Show resolved Hide resolved
pksunkara
pksunkara requested changes Feb 1, 2020
View reviewed changes
Copy link
Member

@pksunkara pksunkara left a comment

Choose a reason for hiding this comment

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

Please actually fix the issue also instead of just reverting the PR. I have made the necessary suggestions. Once you apply them, I will approve and merge the PR.

src/build/app/mod.rs Outdated Show resolved Hide resolved
@pksunkara
Copy link
Member

http://www.tldp.org/LDP/abs/html/exitcodes.html

@CreepySkeleton
Copy link
Contributor

http://www.tldp.org/LDP/abs/html/exitcodes.html

And it doesn't define 64 as "usage" either. yes, it tries to delegate to sysexits.h but a standard is not really standard if nobody uses it.

Can somebody supply an example of a widely received CLI app that actually exits with 64 on usage?

@pksunkara
Copy link
Member

@BurntSushi What do you think?

@BurntSushi
Copy link
Contributor

I would probably go with an exit code of 2 for a usage error.

bors bot added a commit that referenced this pull request Feb 3, 2020
@bors @Dylan-DPC @pksunkara
Merge #1653
3d9f635 
1653: Revert " imp: Add exitcode::USAGE exit code as suggested in #1327" r=pksunkara a=Dylan-DPC

Reverts #1637

Co-authored-by: Dylan DPC <dylan.dpc@gmail.com>
Co-authored-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
@pksunkara pksunkara force-pushed the revert-1637-use-USAGE-error-exit-code branch from 74055a2 to c5eb084 Compare February 3, 2020 11:37
@bors
Copy link
Contributor

bors bot commented Feb 3, 2020

Canceled

@pksunkara
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 3, 2020

🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@CreepySkeleton
Copy link
Contributor

bors r=me

bors bot added a commit that referenced this pull request Feb 3, 2020
@bors @Dylan-DPC @pksunkara
Merge #1653
f981831 
1653: Revert " imp: Add exitcode::USAGE exit code as suggested in #1327" r=me a=Dylan-DPC

Reverts #1637

Co-authored-by: Dylan DPC <dylan.dpc@gmail.com>
Co-authored-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
@bors
Copy link
Contributor

bors bot commented Feb 3, 2020

Build succeeded

@bors bors bot merged commit c5eb084 into master Feb 3, 2020
@bors bors bot deleted the revert-1637-use-USAGE-error-exit-code branch February 3, 2020 13:09
@TeXitoi
Copy link
Contributor

TeXitoi commented Feb 3, 2020

@CreepySkeleton r=me doesn't work really well, that's r+

f981831

@nandahkrishna nandahkrishna mentioned this pull request May 11, 2021
Closed
epage added a commit to epage/clap that referenced this pull request Jul 19, 2021
@epage
fix(exit): Be consistent in exit code
93a097e 
PR clap-rs#1637 switched clap to report `64` on errors and then clap-rs#1653 switch it
to 2, but both missed a case.  This also documents the reason why inline
since I had to go and dig through the history to re-discover the
motivation.
@epage epage mentioned this pull request Jul 19, 2021
Merged
epage added a commit to epage/clap that referenced this pull request Jul 19, 2021
@epage
fix(exit): Be consistent in exit code
580c347 
PR clap-rs#1637 switched clap to report `64` on errors and then clap-rs#1653 switch it
to `2`, but both missed a case.  This also documents the reason why inline
since I had to go and dig through the history to re-discover the
motivation.
epage added a commit to epage/clap that referenced this pull request Jul 19, 2021
@epage
fix(exit): Be consistent in exit code
8b84c05 
PR clap-rs#1637 switched clap to report `64` on errors and then clap-rs#1653 switch it
to `2`, but both missed a case.  This also documents the reason why inline
since I had to go and dig through the history to re-discover the
motivation.
epage added a commit to epage/clap that referenced this pull request Jul 19, 2021
@epage
fix(exit): Be consistent in exit code
6e24c84 
PR clap-rs#1637 switched clap to report `64` on errors and then clap-rs#1653 switch it
to `2`, but both missed a case.  This also documents the reason why inline
since I had to go and dig through the history to re-discover the
motivation.
@tailhook tailhook mentioned this pull request Dec 22, 2021
Merged
@tertsdiepraam tertsdiepraam mentioned this pull request Feb 9, 2022
Closed
2 tasks
@sourcefrog sourcefrog mentioned this pull request Jan 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pksunkara pksunkara pksunkara requested changes

@CreepySkeleton CreepySkeleton CreepySkeleton approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Dylan-DPC-zz @CreepySkeleton @pksunkara @BurntSushi @TeXitoi @Dylan-DPC