Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Improve the contributing guidelines. #760
Improve the contributing guidelines. #760
Changes from 1 commit
0dae824
2c56c8b
99f304f
0bfa77e
034c765
abb5b87
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Here you run a linter. You #757 you told me that you don't.
#757 is not fixed. IMHO it would be a good idea to mention your code style even if you do not care about code style. Also the latter is a "tell".
What I do not understand is why pyflakes do not throw any errors. I couldn't find a pyflakes config somewhere. But the code do not follow PEP8 and should throw 100 of errors.
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.
This is only about line 47-48.
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 meant we don't have a formatter, like black. pyflakes check for errors in the code, not the formatting.
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.
"errors in the code" and "formatting" in Python is often the same because the format (e.g. indention) is part of the logic.
Anyway: I mixed up "pyflakes" with "flake8". Both are linters but "pyflakes" is of the "lazy type" and by design do not check for PEP8 or similar problems. Now I understand why it runs without errors. It is kind of a "minimal linter".
What kind of "errors" would pyflakes find? It's docu do not list errors.
But even this should be an element of the contrib docu. "No explicit coding style just following the minimal rules of pyflakes" (or something like this). But don't you have rules about naming variables for example; camel case, upper case letters only, etc pp? Do you accept PRs with line length of 120 characters?
The key of such pedantic details is that some contributors won't start a PR when they have to be scared that the maintainer will reject the PR because of (unknown) style reasons. Maintainers should state very clear what they want and what not. Otherwise contributors wouldn't risk to burn their resources.
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.
Pyflakes will catch unused imports, name error and other relatively obvious mistakes. For the coding style I think we can just link the PEP8 guidelines. Pydoctor is about 20 years old, and we have vendored some code from other packages as well: so we can’t really enforce the same style everywhere. I’ll accept long line if it’s justified, it’s up to the reviewer to request refactors if some proposed change is very ugly.
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.
This is a nice explanation and could fit well into a contrib file.