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

Change exit code for CLI flag error #7134

Merged
merged 7 commits into from Aug 15, 2023
Merged

Change exit code for CLI flag error #7134

merged 7 commits into from Aug 15, 2023

Conversation

ybiquitous
Copy link
Member

Which issue, if any, is this issue related to?

Closes #4989

Is there anything in the PR that needs further explanation?

The reason I choose the exit code 64 is defined sysexits.h as EX_USAGE:

#define EX_USAGE	64	/* command line usage error */

If your OS is Linux-based or macOS, you may find sysexits.h by the following command:

find / -name sysexits.h -type f

@changeset-bot

This comment was marked as resolved.

Sign in to view
@ybiquitous ybiquitous marked this pull request as ready for review August 14, 2023 13:53
ybiquitous
ybiquitous commented Aug 14, 2023
View reviewed changes
lib/constants.mjs Outdated Show resolved Hide resolved
Mouvedia
Mouvedia reviewed Aug 14, 2023
View reviewed changes
docs/user-guide/cli.md Outdated Show resolved Hide resolved
jeddy3
jeddy3 reviewed Aug 14, 2023
View reviewed changes
Copy link
Member

@jeddy3 jeddy3 left a comment •
edited

Choose a reason for hiding this comment

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

Looking good! Thank you for picking up these related CLI issues.

I've made two suggestions.

lib/constants.mjs Outdated Show resolved Hide resolved
docs/user-guide/cli.md Outdated Show resolved Hide resolved
ybiquitous
ybiquitous commented Aug 15, 2023
View reviewed changes
lib/cli.mjs Show resolved Hide resolved
jeddy3
jeddy3 approved these changes Aug 15, 2023
View reviewed changes
Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

Thank you for all the clarifications.

I've suggested one minor change to the migration guide to align with our house style.

LGTM, otherwise!

docs/migration-guide/to-16.md Outdated Show resolved Hide resolved
ybiquitous and others added 2 commits August 15, 2023 18:34
@ybiquitous @jeddy3
Update docs/migration-guide/to-16.md
88147f6 
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
@ybiquitous
Fix lint error in to-16.md
42ab635
Mouvedia
Mouvedia approved these changes Aug 15, 2023
View reviewed changes
Copy link
Member

@Mouvedia Mouvedia left a comment

Choose a reason for hiding this comment

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

Apart from the missing import, LGTM.

@ybiquitous ybiquitous merged commit c3124b3 into v16 Aug 15, 2023
12 of 14 checks passed
@ybiquitous ybiquitous deleted the change-cli-exit-codes branch August 15, 2023 13:49
@ybiquitous
Copy link
Member Author

Thanks for the review!

@ybiquitous ybiquitous mentioned this pull request Aug 15, 2023
Closed
ybiquitous added a commit that referenced this pull request Aug 27, 2023
@ybiquitous @jeddy3
Change exit code for CLI flag error (#7134)
de9299d 
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
ybiquitous added a commit that referenced this pull request Aug 27, 2023
@ybiquitous @jeddy3
Change exit code for CLI flag error (#7134)
93d6b9c 
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
ybiquitous added a commit that referenced this pull request Sep 28, 2023
@ybiquitous @jeddy3
Change exit code for CLI flag error (#7134)
355b3ce 
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
ybiquitous added a commit that referenced this pull request Oct 19, 2023
@ybiquitous @jeddy3
Change exit code for CLI flag error (#7134)
daefd6a 
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mouvedia Mouvedia Mouvedia approved these changes

@jeddy3 jeddy3 jeddy3 approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ybiquitous @Mouvedia @jeddy3