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.

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

Fix file argument #154

Merged
merged 4 commits into from
Sep 12, 2023
Merged

Conversation

starswaterbrook
Copy link
Contributor

This fix should allow omitting --file argument (prints the help message if so), while still allowing to pass arguments like --verbose or --version without crashing.

Signed-off-by: Maciej <mwkm00@gmail.com>
@jspeed-meyers
Copy link
Collaborator

Thank you, @mwkm00!

@jspeed-meyers
Copy link
Collaborator

@mwkm00, it looks like the black formatting linter is failing.

More info on black is here: https://github.com/psf/black

Would you mind fixing that before I review this?

Signed-off-by: Maciej <mwkm00@gmail.com>
@starswaterbrook
Copy link
Contributor Author

Should be good now, I assumed exit code 1 would be acceptable after not delivering file arg, but the program goes to the help message and exits gracefully so exit code 0 is probably the way to go. Tests should be fixed now.

Copy link
Collaborator

@jspeed-meyers jspeed-meyers left a comment

Choose a reason for hiding this comment

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

I like this approach. Nice. Thank you!

@jspeed-meyers
Copy link
Collaborator

But the black linter is still failing. hmmm 🤔

@starswaterbrook
Copy link
Contributor Author

My bad, formatting. Thanks for patience.

@jspeed-meyers
Copy link
Collaborator

@vargenau, is that you were thinking?

@vargenau
Copy link
Contributor

Hi,

Yes, it works correctly now.

Thank you for the quick fix.

@jspeed-meyers
Copy link
Collaborator

@goneall, would you please take a quick look too? 🙏

Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

LGTM

@jspeed-meyers jspeed-meyers merged commit 3c19702 into spdx:main Sep 12, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: test the presence of the --file argument if another argument is present
4 participants