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

Exit bug #102

Merged
merged 7 commits into from
Sep 25, 2024
Merged

Exit bug #102

merged 7 commits into from
Sep 25, 2024

Conversation

sepandhaghighi
Copy link
Owner

Reference Issues/PRs

What does this implement/fix? Explain your changes.

  • Exit bug fixed

Any other comments?

Copy link

codecov bot commented Sep 22, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.14%. Comparing base (231322a) to head (2d38171).
Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
nafas/functions.py 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #102      +/-   ##
==========================================
- Coverage   96.57%   96.14%   -0.43%     
==========================================
  Files           2        2              
  Lines         204      207       +3     
  Branches       24       25       +1     
==========================================
+ Hits          197      199       +2     
  Misses          5        5              
- Partials        2        3       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sepandhaghighi sepandhaghighi self-assigned this Sep 22, 2024
@sepandhaghighi sepandhaghighi added bug Something isn't working minor labels Sep 22, 2024
@sepandhaghighi sepandhaghighi added this to the nafas v0.8 milestone Sep 22, 2024
@sepandhaghighi sepandhaghighi marked this pull request as ready for review September 22, 2024 22:25
Copy link
Collaborator

@sadrasabouri sadrasabouri left a comment

Choose a reason for hiding this comment

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

Minor changes needed.

nafas/functions.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@sadrasabouri sadrasabouri left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +75 to +78
>>> get_input_standard(test_keyboard_interrupt)
Traceback (most recent call last):
...
SystemExit
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also have EXIT_MESSAGE content here?
That's not a big deal, just to make sure everything is tested as much as possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than that, it's good. If you don't think adding such a thing is necessary you can merge it.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Test system behavior is a bit weird here. I think it's because of the sys.exit().

@sadrasabouri sadrasabouri merged commit 4508cde into dev Sep 25, 2024
45 checks passed
@sadrasabouri sadrasabouri deleted the exit_bug branch September 25, 2024 06:01
@sepandhaghighi sepandhaghighi mentioned this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants