Skip to content
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

CI: Lint docs #7479

Merged
merged 7 commits into from Nov 27, 2023
Merged

CI: Lint docs #7479

merged 7 commits into from Nov 27, 2023

Conversation

szepeviktor
Copy link
Contributor

There is a tool called sphinx-lint

- echo 'Hello!';
-<TAB>echo 'Hello!';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reStructuredText converts TAB characters to spaces :(
https://cs.symfony.com/doc/rules/whitespace/indentation_type.html 👀

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Nov 26, 2023

I see! Documentation is generated from FixerDefinition.
For example definition for method_argument_space deliberately contains trailing spaces.
💡

@Wirone
Copy link
Member

Wirone commented Nov 26, 2023

Yeah, this one may be tricky, since docs are generated (with composer docs script), and there are tests that verify consistency between source code and generated RSTs. But it looks promising, it's great to spot such things like in this PR's diff 👌.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Nov 26, 2023

@Wirone Please help me code these two.

doc/ruleSets/PER-CS1.0.rst:5: default role used (hint: for inline literals, use double backticks) (default-role)
doc/ruleSets/PER-CS1.0Risky.rst:5: default role used (hint: for inline literals, use double backticks) (default-role)

In rst world we need two backticks.

**This ruleset is deprecated** in favour of `@PER-CS2.0:risky`.

@Wirone
Copy link
Member

Wirone commented Nov 26, 2023

@szepeviktor I am AFK right now, reviewing from mobile. Now I am going to be mostly offline, I can look in the evening if the problem remains 🙂.

@keradus keradus changed the title chore: Lint docs CI: Lint docs Nov 26, 2023
@Wirone
Copy link
Member

Wirone commented Nov 27, 2023

@szepeviktor I've changed those 2 sets locally (double backtick), generated docs (composer docs) and whole QA suite (composer qa) is green. I've reproduced Sphinxlint's CI job locally (inside of docker run --rm -it -w /fixer -v pwd:/fixer ubuntu:22.04 bash) and it looks OK:

root@996bbd4a9326:/fixer# apt-get update && apt-get install python3 pip git && pip install --user sphinx-lint && mv /root/.local/bin/sphinx-lint /usr/local/bin/

root@996bbd4a9326:/fixer# git --version
git version 2.34.1
root@996bbd4a9326:/fixer# sphinx-lint --version
sphinx-lint 0.9.0
root@996bbd4a9326:/fixer# git ls-files --cached -z -- '*.rst' | xargs --null -- python3 -m sphinxlint --enable all --disable trailing-whitespace --max-line-length 2000
No problems found.

I'll just push the changes here (with a rebase).

@coveralls
Copy link

Coverage Status

coverage: 94.918% (-0.004%) from 94.922%
when pulling 85367e4 on szepeviktor:rst
into 39596a3 on PHP-CS-Fixer:master.

Copy link
Member

@Wirone Wirone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff @szepeviktor 👏 . LGTM.

@Wirone Wirone merged commit cee6945 into PHP-CS-Fixer:master Nov 27, 2023
27 checks passed
@szepeviktor szepeviktor deleted the rst branch November 27, 2023 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants