-
Notifications
You must be signed in to change notification settings - Fork 7
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
Replace darglint
with pydoclint
#124
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
part:tests
Affects the unit, integration and performance (benchmarks) tests
label
Aug 25, 2023
This comment was marked as outdated.
This comment was marked as outdated.
llucax
changed the title
Replace
Replace Aug 28, 2023
darglint
with pydocstyle
darglint
with pydoclint
This comment was marked as outdated.
This comment was marked as outdated.
llucax
added
part:docs
Affects the documentation
part:tooling
Affects the development tooling (CI, deployment, dependency management, etc.)
type:enhancement
New feature or enhancement visitble to users
part:nox
Affects the configuration of nox
part:cookiecutter
Affects the generation of projects using cookiecutter
labels
Aug 28, 2023
This comment was marked as outdated.
This comment was marked as outdated.
github-actions
bot
removed
part:docs
Affects the documentation
part:tooling
Affects the development tooling (CI, deployment, dependency management, etc.)
part:nox
Affects the configuration of nox
part:cookiecutter
Affects the generation of projects using cookiecutter
labels
Aug 28, 2023
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Bug in pydoclint fixed! 🎉 This is now passing tests! |
Enabled auto-merge. |
The `darglint` project is not maintained anymore, and even when there is a fork that is trying to keep it alive, it is still extremely slow. The new project `pydoclint` was created recently but advancing rapidly, and already in better shape than `darglint` and way faster (0.2s vs 20s for the SDK, 100x improvement). To be able to add ignore comments in the code, `pydoclint` needs to be run via `flake8`. Because we need to run `flake8` already, and `pydocstyle` also can run via `flake8`, we are merging both in one `flake8` call, so we only need to read the files once. As a side effect, we are also enabling other base `flake8` checks. There is probably some overlap with `pylint`, but `flake8` is very fast anyway, so it shouldn't be noticed (for the SDK, `flake8` basic checks + `pydocstyle` + `pydoclint` runs in 0.6s and `pylint` alone runs in 15s). At some point we might want to disable the duplicated checks in `pylint` to see if it speeds up `pylint`. This commit also renames the `dev-docstrings` optional dependency to `dev-flake8` because now is not checking docstrings exclusively. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
`pydocstyle` can now use the built-in `tomllib`, and now the configuration is read indirectly by `flake8` anyway. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Some are just `flake8` base checks, and some are found by `pydoclint` (issues that `darglint` didn't catch): * Line too long. * Ununsed imports. * Ambiguous variable name (`l` could be read as L or the number one with some fonts). * Missing argument documentation. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
It requires some extra steps when updating. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Marenz
reviewed
Aug 29, 2023
@@ -16,6 +16,8 @@ | |||
|
|||
### Cookiecutter template | |||
|
|||
- CI: The `nox` job now uses a matrix to run the different `nox` sessions in parallel. If you use branch projection with the `nox` job you need to update the rules to include each matrix job. |
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.
oh nice
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.
2x speedup for this repo CI.
Marenz
approved these changes
Aug 29, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
part:tests
Affects the unit, integration and performance (benchmarks) tests
type:enhancement
New feature or enhancement visitble to users
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.
The
darglint
project is not maintained anymore, and even when there is a fork that is trying to keep it alive, it is still extremely slow.The new project
pydoclint
was created recently but advancing rapidly, and already in better shape thandarglint
and way faster.To be able to add ignore comments in the code,
pydoclint
needs to be run viaflake8
. Because we need to runflake8
already, andpydocstyle
also can run viaflake8
, we are merging both in oneflake8
call, so we only need to read the files once.Performance running on the for the SDK
src
directory only:As a side effect, we are also enabling other base
flake8
checks. There is probably some overlap withpylint
, butflake8
is very fast anyway, so it shouldn't be noticed (for the SDK,flake8
basic checks +pydocstyle
+pydoclint
runs in 0.6s andpylint
alone runs in 15s). At some point we might want to disable the duplicated checks inpylint
to see if it speeds uppylint
.This commit also renames the
dev-docstrings
optional dependency todev-flake8
because now is not checking docstrings exclusively.