astral-sh
Add markdownlint, prettier, and dev ruff to pre-commit
Hi,
I propose adding markdownlint and Prettier to pre-commit.
I already added markdownlint but I did not add Prettier because dev-generate-all is incompatible with it.
We would have to run Prettier inside dev-generate-all.
To do this we either require the developer to have node installed on their machine or use nodeenv which only requires Python to create a Node virtual environment where we would run Prettier.
Prettier would be really useful to format ruff.schema.json as it is required by schemastore.org.
Also maybe we should consider using for the ruff hook cargo run with repo: local instead of ruff-pre-commit which sometimes fails because the version for the latest release is not out yet.
Cargo format is part of our GitHub actions pipeline. If markdownlint is added, any reason to not enforce it there?
We could use pre-commit.ci that would run on pull-request to do this.
Certainly an option. There is also a markdownlint github action: https://github.com/actionshub/markdownlint (that does not require to install a github application)
I already added markdownlint but I did not add Prettier because dev-generate-all is incompatible with it.
Can you expand on this? How is it incompatible?
Dev generate all will generate the different file which will be then formated by prettier. Pre-commit will always fail at the one of the hook as the output of prettier is different from dev generate all. For it to be compatible it would need that dev generate all generate the different files already well formated.
To do this we either require the developer to have node installed on their machine or use nodeenv which only requires Python to create a Node virtual environment where we would run Prettier.
I'd really rather not add Node as a dev dependency.
Prettier would be really useful to format ruff.schema.json as it is required by schemastore.org.
I think the better approach would be to implement https://github.com/SchemaStore/schemastore/issues/2731 then our schema file would no longer have to be part of the schemastore repository.
Yeah I'd like to avoid adding Node as a dependency. I'm happy to just do this manually from time to time, even though it's not a great practice.
I added prettier in pre-commit. I had to:
- Ignore formatting for the table (c.f. prettier/prettier-vscode#762)
- Use
-as bullet as prettier does not support*bullet (c.f. prettier/prettier#4440). prettier option philosophy is to have few option so that we do not debate on which style to chose.By far the biggest reason for adopting Prettier is to stop all the ongoing debates over styles. Yet the more options Prettier has, the further from the above goal it gets. The debates over styles just turn into debates over which Prettier options to use.
I added prettier in pre-commit.
Does this require that the user have Node installed? Or how does it work?
(I don't feel strongly about those individual style decisions, like * vs. -. Even if I have preferences, it's not a blocker for me.)
I added prettier in pre-commit.
Does this require that the user have Node installed? Or how does it work?
No, it does not require Node. If pre-commit is installed it will run the prettier hook with nodeenv but the developer only need to have pre-commit installed and nothing else.
For the CI, I would suggest using pre-commit.ci to run pre-commit. To avoid checking twice fmt, clippy, and dev-generate-all, we can edit .pre-commit-config.yaml to skip them when in CI.
Like this validate-pyproject, prettier, markdownlint-fix, and ruff hooks would run on the pull request.
Which is not the case currently.
Using pre-commit.ci only needs .pre-commit-config.yaml and avoids maintaining a separate GitHub Action.
Candidly, while I definitely appreciate the motivation, I'm probably not going to enable pre-commit CI right now, since I don't use pre-commit personally!
@JonathanPlasse - I'd like to pull in some of these changes, but exclude Prettier for now. Would you like me to do it in a separate PR, or push to this branch?