-
Notifications
You must be signed in to change notification settings - Fork 10
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
Stricter linting and CI/CD #35
Comments
Excellent - thanks for reporting this, @edmondop ! Would love this as a contribution, if you are game for it. Otherwise, just let us know and we'll push to get the linter step turned on. Here is a draft PR, if you wanted to work off that https://github.com/log10-io/log10/pull/38/files |
Thanks @nqn if you are ok, I will proceed using wemake-python-styleguide. I have an "industrial" setup that I use and I am pretty happy for it, it's pretty strict. Are you ok with it? |
That sounds great! |
|
Hi @edmondop ! Is there a way to apply an auto formatter which adheres to that syntax style, that you know of? |
Yes, I have a setup with that see https://github.com/log10-io/log10/pull/54/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R58-R61 I just didn't want to make further changes before confirming people are ok with that, it's a big change |
At the moment we do have only a release pipeline, but it would be worth to have at least a linting pipeline to verify the code is somehow correct. Such a pipeline would run an opinionated linter, such as
wemake-python-styleguide
and encourage better practices.For example, this code
presents a couple of problems. First of all, given the large number of arguments that have the same type, it's easy to make mistakes in inverting the error, and for those scenarios Python introduced keyword-only named arguments
which would force client code to invoke it like so:
Also, the code would not pass mypy, because None are not valid types for str. It should rather be
Optional[str]
like so:The text was updated successfully, but these errors were encountered: