-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: warn on microsecond granularity and empty string summary #191
feat: warn on microsecond granularity and empty string summary #191
Conversation
6d348c2
to
baf2887
Compare
52a64f0
to
c142a6f
Compare
468caa6
to
a47ff3f
Compare
4125ab0
to
37f7bb5
Compare
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.
Thanks a lot for the feedback!
BTW, the code-base was pretty nice to dive into, kudos 👍
A few "line too long", I'll take a look. BTW, would you be open to adding a formatter (e.g. ruff/black?). Should make this easier 👍 |
Yeah, I'm a bit against black in general for some of its style decisions and lack of context awareness which often breaks readability. I see its benefits for bigger teams but this project doesn't have one. So I'd prefer readability here. But I agree that this workflow should be improved. I think we can use the flake8 pre-commit hook for that. |
LGTM. Thank you for the PR and finding the issues! Merging to dev until release. |
Fixes #189 and fixes #190.