-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
Rename em_keyboard
package to em
and refactor
#166
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #166 +/- ##
==========================================
- Coverage 95.56% 92.85% -2.71%
==========================================
Files 2 3 +1
Lines 203 126 -77
==========================================
- Hits 194 117 -77
Misses 9 9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@hukkin What do you think of this? |
I like this! Something to also consider is making def main(args: Sequence[str]) -> int:
... and then in def main() -> NoReturn:
exit_code = em_keyboard.cli.main(sys.argv[1:])
raise SystemExit(exit_code)
if __name__ == "__main__":
main() Oh and The benefit of this is that it's very easy to test |
Another thing to consider: rename the import package as Then one could do |
Yes, that reminds me of this pattern: https://pythontest.com/testing-argparse-apps/ I used something similar for The mocking is a pain, will be good to remove that. And yes, |
__init__
into cli
and __main__
em_keyboard
package to em
and refactor
OK, ready! |
pyproject.toml
Outdated
@@ -6,7 +6,7 @@ requires = [ | |||
] | |||
|
|||
[project] | |||
name = "em-keyboard" | |||
name = "em" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, of course, reverted.
I'll email them asking if they would like to transfer the name :)
This is much nicer than before! |
Thanks for the review! |
The
__main__
allows you to runpython -m em_keyboard
, making it easier to run/test with another interpreter.Another reason is to mirror my other CLIs (for example, pypistats, norwegianblue, pepotron, linkotron and tinytext), making it easier to diff/compare and maintain.
Because em_keyboard doesn't have a public API, perhaps we could move most of
__init__.py
intocli.py
as well?