-
Notifications
You must be signed in to change notification settings - Fork 177
chore: shorten docstrings and single-line comments to 79 (ruff w505: doc-line-too-long) #2180
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
chore: shorten docstrings and single-line comments to 79 (ruff w505: doc-line-too-long) #2180
Conversation
Reproduce with:
and drop the |
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 my word @felix314159, we've embarked on quite the mission with this one π
I did some manual review. Three are some slight differences in formatting in the docstrings (e.g., indentation), but I think we can live with it.
There are two broken things still:
- Lint-related: Broken formatting in docstrings that contain unordered lists (i.e., they start with
-
). - Docflow-related and lint-related): Some of the fixes here break formatting in the auto-generated test case reference html section. In general, the use of the "abstract" admonition in test module docstrings for the generated test case reference imposes an unnecessarily strict syntax for docstrings which is not Python native. It is annoying for developers and error prone. I think it'd be best just to remove it here.
I need to take a break from this one for a bit. Perhaps you could have a look at 1 @felix314159? Once recharged and you're ready for a rest, I can handle 2.
For 2. I would remove all "abstract" admonitions and use this format:
"""
One line summary.
Longer description, if available, if not, the docstring is a single-liner.
"""
And I think we should continue to ignore D200 post-weld, see comments here:
- ethereum/execution-specs#1428 (comment)
- ethereum/execution-specs#1428 (comment) (I would love to enable D205, but I think it's too much work in one bulk PR - the solution to 2. above is so that we could, and imo should, enable it for
./tests/
).
Happy to align, when you're ready to pick this up!
c601f1d
to
2a7a47c
Compare
fc960fb
to
5981abd
Compare
e96a3cd
to
bff747c
Compare
confirmed with hasher that
|
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 :)
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 all your work on this @felix314159 !
ποΈ Description
This required two separate scripts, a bunch of manual fixes and we should ignore rule D200: unnecessary-multiline-docstring).
Please thoroughly review if CI passes and u find the time.
Both of these commands gives me
All checks passed!
:uv run ruff check src tests .github
uv run ruff check --select W505 --config 'lint.pycodestyle.max-doc-length = 79' src tests .github
The command
uv run ruff format --check src tests .github
gives me
720 files already formatted
.Mypy still passes:
uv run mypy
π Related Issues or PRs
N/A.
β Checklist
tox
checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
type(scope):
.mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_from
marker.