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

Add CLI #479

Merged
merged 17 commits into from
Feb 13, 2025
Merged

Add CLI #479

merged 17 commits into from
Feb 13, 2025

Conversation

JannisNe
Copy link
Collaborator

Add a CLI using typer. The dependencies will not be installed by default but are included as extras cli. So to run the CLI nuztf has to be installed via poetry install -E cli.

The command is nuztf and documentation is provided in the help message nuztf --help and the README.

@coveralls
Copy link

coveralls commented Feb 10, 2025

Coverage Status

coverage: 73.615% (+0.6%) from 72.978%
when pulling 9758a83 on cli
into 5d4c681 on main.

Copy link
Member

@robertdstein robertdstein left a comment

Choose a reason for hiding this comment

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

Looks good. One request: can you either exclude this from coveralls calculations, or add a test?

Copy link
Member

@robertdstein robertdstein left a comment

Choose a reason for hiding this comment

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

Sorry, one more request: rather than a real tempfile can you use https://docs.python.org/3/library/tempfile.html ? Then even if the test is interrupted the file gets cleaned up. You can use it within a with statement.

@JannisNe
Copy link
Collaborator Author

JannisNe commented Feb 13, 2025

I actually started using the tempfile package. However, I found it then useful if the test gcn is still there if the test fails to check what exactly changed.

@robertdstein robertdstein self-requested a review February 13, 2025 18:59
Copy link
Member

@robertdstein robertdstein left a comment

Choose a reason for hiding this comment

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

Makes sense!

@JannisNe JannisNe enabled auto-merge (squash) February 13, 2025 20:52
@JannisNe JannisNe disabled auto-merge February 13, 2025 20:52
@JannisNe JannisNe merged commit b145bc5 into main Feb 13, 2025
7 checks passed
@JannisNe JannisNe deleted the cli branch February 13, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants