[doc] Apply black on the documentation where it makes sense #8650
Conversation
Pierre-Sassoulas
added
Documentation 📗
Maintenance
Pierre-Sassoulas
force-pushed
the
apply-black-on-relevant-doc
branch
from
1f11514 to
501c1e0
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8650 +/- ##
=======================================
Coverage 95.81% 95.81%
=======================================
Files 174 174
Lines 18337 18337
=======================================
Hits 17570 17570
Misses 767 767 |
| # Should be in sync with doc/data/ruff.toml | ||
| args: [--safe, --quiet, -l 103] | ||
| files: doc/data/messages/ | ||
| exclude: | |
There was a problem hiding this comment.
Can't we re-use this like we do with &fixtures and pass it in the pre-commit config?
There was a problem hiding this comment.
What need to be in sync is the -l 103, black only looks at pyproject.toml so we have to give the option like this. (Alternatively we could use the default value, it's not super important.).
I don't think the exclude can be reused, this is the list of the message that black can fix automatically, it's really specific to black.
It's also a very useful list btw ! I'm considering to recover it automatically and generalize it to autofixxer (isort, autoflake, and ruff), in order to use this information directly in our doc for #8579
There was a problem hiding this comment.
I.e. if applying a tool on doc/data makes the tests fail on a m/my-message/bad.py file, then the tool autofix my-message. We might have to use it on functional tests instead for complicated fixes like ruff or autoflake to check which use cases is autofixed or not, but for black on the doc is enough and you get the idea.
There was a problem hiding this comment.
Yeah, let's use the default then. Less config == better imo
There was a problem hiding this comment.
I'm going to need to rebase and rewrite black from zero, it's not THAT good with comment after all 😄
There was a problem hiding this comment.
Shall we merge this and do another PR that removes the config?
There was a problem hiding this comment.
No, it breaks the comments to go back to 88 char per line after doing 103 char per line, it's a lot easier to do it from scratch again :)
Pierre-Sassoulas
force-pushed
the
apply-black-on-relevant-doc
branch
from
0034c3c to
e4f17ec
Compare
|
Sorry for the noise, should be ready for review again. |
.pre-commit-config.yaml
Outdated
| files: doc/data/messages/ | ||
| exclude: | | ||
| (?x)^( | ||
| doc/data/messages/r/redundant-u-string-prefix/bad.py| |
There was a problem hiding this comment.
Shall we make this alphabetized?
Pierre-Sassoulas
force-pushed
the
apply-black-on-relevant-doc
branch
from
876e5b8 to
e7ba0a0
Compare
.pre-commit-config.yaml
Outdated
| @@ -52,6 +52,30 @@ repos: | |||
| - id: black | |||
| args: [--safe, --quiet] | |||
| exclude: *fixtures | |||
| - repo: https://github.com/psf/black | |||
There was a problem hiding this comment.
One final nit, can't this be a separate hook in the repo above? Or this is not how pre-commit works?
There was a problem hiding this comment.
I think it's a yaml issue actually, we can't use *fixtures with another regex (or at least I'm not knowledgeable enough to do it).
There was a problem hiding this comment.
Pushed a small change, let me know what you think!
There was a problem hiding this comment.
Same thing could be done for ruff actually
There was a problem hiding this comment.
I'll do that next time there's a pre-commit autoupgrade, (should be soon).
Type of Changes
Description
Refs: #8630 (comment)