-
Notifications
You must be signed in to change notification settings - Fork 3
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
Modernize repo #304
Modernize repo #304
Conversation
033d708
to
67ded27
Compare
2ccaadf
to
17186aa
Compare
IMO, striving for a 100% code coverage metric makes us focus on the wrong things. As long as our business logic is covered (which should be >80% in projects that aren't just all IO), I'm happy. |
|
||
# Run chosen subcommand. | ||
if args.which == "dump": | ||
if args.full: |
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.
nit: It always feels like I should have another function when my if has more than one thought in it. ex:
if args.which == "dump"
doDump(...)
elif args.which == "load"
doLoad(...)
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.
+1
This is a bit out of scope, the project probably has a lot more to refactor and polish
Looks good, we just need to fix the CI checks |
requirements.txt
bypip-compile
master
tomain
Things lost with this PR: