-
Notifications
You must be signed in to change notification settings - Fork 23
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
Update .gitignore #176
Update .gitignore #176
Conversation
makefile
Outdated
@@ -19,12 +19,12 @@ format: | |||
|
|||
lint: | |||
python -m flake8 $(PKG) | |||
python -m mypy -p $(PKG) |
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.
I think that mypy checks are suitable for this section since type checks are part of linting stage.
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.
I disagree with this, because mypy is testing the type annotations and finding errors in code, rather than just linting for best practices.
makefile
Outdated
python -m ufmt check $(PKG) | ||
|
||
test: | ||
test: lint |
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.
@amyreese , the rationale for this is not running tests on the code that foes not pass quality gates. The checks from mypy
is a sort of linting stage. If the idea is grasped right, the test rule becomes depending on the lint rule that should be run before.
Let's discuss this addendum.
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.
I very much dislike this, because during development of new features, it's more important to know if tests are passing than whether or not all of the lints have been taken care of. Not being able to validate functionality of code because there's extra import or unused value is a really bad UX.
@amyreese , could the |
351dd73
to
7026ff2
Compare
I went ahead and reverted the makefile changes, as well as the markdown changes for now. Considering #183 as that is focused on sphinx compat rather than opinionated formatting. |
I also removed that pylint config. Thank you for catching that. |
Updating the project metadata files
For not having some features from the PR #171 completely gone, the desirable ones are moved here.
Changes in short:
.gitignore
file is updated according to the Python template from this repomakefile
to ensure the chain of quality checks@amyreese , are these changes welcome and good enough to be approved and merged?