Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor: switch args style #72

Merged
merged 5 commits into from
Jan 9, 2025
Merged

Refactor: switch args style #72

merged 5 commits into from
Jan 9, 2025

Conversation

thekaveman
Copy link
Member

Closes #71

@thekaveman thekaveman requested a review from a team as a code owner December 6, 2024 21:12
@thekaveman thekaveman added the cli New command line interface feature implementation or refactor label Dec 6, 2024
Copy link

github-actions bot commented Dec 6, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  littlepay
  config.py
  main.py
  littlepay/commands
  switch.py
Project Total  

This report was generated by python-coverage-comment-action

was using the old switch syntax, and was disabled anyway
lalver1
lalver1 previously approved these changes Dec 18, 2024
Copy link
Member

@lalver1 lalver1 left a comment

Choose a reason for hiding this comment

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

This is a nice change and everything worked as expected 👍
The only thing I noticed was that we may be missing a test for when switch raises a ValueError. I don't think it's super important but if we think it'll help we could add one more test to test_switch.py just to make sure line 7 is covered:

def test_switch_missing_args():

    with pytest.raises(ValueError):
        switch()

@thekaveman
Copy link
Member Author

This is a nice change and everything worked as expected 👍 The only thing I noticed was that we may be missing a test for when switch raises a ValueError. I don't think it's super important but if we think it'll help we could add one more test to test_switch.py just to make sure line 7 is covered:

def test_switch_missing_args():

    with pytest.raises(ValueError):
        switch()

Thanks for this suggestion. Added in edefa93

Copy link
Member

@lalver1 lalver1 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!

Copy link
Member

@angela-tran angela-tran left a comment

Choose a reason for hiding this comment

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

This looks good to me and will be really nice to use 🙏

@thekaveman thekaveman merged commit 1a940c4 into main Jan 9, 2025
3 checks passed
@thekaveman thekaveman deleted the refactor/switch-args branch January 9, 2025 20:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cli New command line interface feature implementation or refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make switch easier with flags instead of positionals
3 participants