-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Docs][CI] Investigate linting C++ and other docs in CI #39990
Comments
take |
I did a bit of research and I think sphinx-lint might work here. I first checked whether sphinx had anything in-built and I didn't find anything. There are reStructuredText linters but at least one of those doesn't understand Sphinx directives so I think we need something Sphinx-aware here. Another option would be to implement checks using one or more shell utilities but that's probably not a good route to go down. sphinx-lint has a number of checks that can be disabled or enabled selectively, and it uses exit codes to indicate a clean run so it would be easy to adapt into CI. The full list of checks are:
And it finds quite a few errors in the sphinx-lint output against main
It mostly found trailing whitespace issues but I am going to look into the other ones they do what we want. After that I'll work up a PR that,
|
Looks useful!
This one seems to be a false positive. The other ones of this type might be similar, so I suppose one solution would be to just disable the full rule. |
It would also be nice, if we add it to CI, if this could also be integrated in the pre-commit hook |
This is mostly done and I put up a draft PR at #40022. I'm seeing some strange behavior with the pre-commit hook that I don't understand yet so I'll keep working on that. |
### What changes are included in this PR? This adds developer tooling to the repo for linting the docs by adding the sphinx-lint tool to archery and our pre-commit hooks. In both locations, only two rules are enabled at the moment (Discussed in #40006): `trailing-whitespace` and `missing-final-newline`. This PR also fixes the individual issues covered by the new tooling. ### Are these changes tested? Yes, though manually. I tested this works by running `archery lint --docs` and `pre-commit` without and without changes that should get caught by the rules. It works as expected. ### Are there any user-facing changes? Yes, 1. Developers that use pre-commit hooks will see a change in behavior when they modify docs 2. Developers using archery will see a new --docs option in `archery lint` 3. Developers working on the docs may see CI failures related to the new checks * Closes: #39990 * GitHub Issue: #39990 Authored-by: Bryce Mecum <[email protected]> Signed-off-by: Bryce Mecum <[email protected]>
…0022) ### What changes are included in this PR? This adds developer tooling to the repo for linting the docs by adding the sphinx-lint tool to archery and our pre-commit hooks. In both locations, only two rules are enabled at the moment (Discussed in apache#40006): `trailing-whitespace` and `missing-final-newline`. This PR also fixes the individual issues covered by the new tooling. ### Are these changes tested? Yes, though manually. I tested this works by running `archery lint --docs` and `pre-commit` without and without changes that should get caught by the rules. It works as expected. ### Are there any user-facing changes? Yes, 1. Developers that use pre-commit hooks will see a change in behavior when they modify docs 2. Developers using archery will see a new --docs option in `archery lint` 3. Developers working on the docs may see CI failures related to the new checks * Closes: apache#39990 * GitHub Issue: apache#39990 Authored-by: Bryce Mecum <[email protected]> Signed-off-by: Bryce Mecum <[email protected]>
…0022) ### What changes are included in this PR? This adds developer tooling to the repo for linting the docs by adding the sphinx-lint tool to archery and our pre-commit hooks. In both locations, only two rules are enabled at the moment (Discussed in apache#40006): `trailing-whitespace` and `missing-final-newline`. This PR also fixes the individual issues covered by the new tooling. ### Are these changes tested? Yes, though manually. I tested this works by running `archery lint --docs` and `pre-commit` without and without changes that should get caught by the rules. It works as expected. ### Are there any user-facing changes? Yes, 1. Developers that use pre-commit hooks will see a change in behavior when they modify docs 2. Developers using archery will see a new --docs option in `archery lint` 3. Developers working on the docs may see CI failures related to the new checks * Closes: apache#39990 * GitHub Issue: apache#39990 Authored-by: Bryce Mecum <[email protected]> Signed-off-by: Bryce Mecum <[email protected]>
…0022) ### What changes are included in this PR? This adds developer tooling to the repo for linting the docs by adding the sphinx-lint tool to archery and our pre-commit hooks. In both locations, only two rules are enabled at the moment (Discussed in apache#40006): `trailing-whitespace` and `missing-final-newline`. This PR also fixes the individual issues covered by the new tooling. ### Are these changes tested? Yes, though manually. I tested this works by running `archery lint --docs` and `pre-commit` without and without changes that should get caught by the rules. It works as expected. ### Are there any user-facing changes? Yes, 1. Developers that use pre-commit hooks will see a change in behavior when they modify docs 2. Developers using archery will see a new --docs option in `archery lint` 3. Developers working on the docs may see CI failures related to the new checks * Closes: apache#39990 * GitHub Issue: apache#39990 Authored-by: Bryce Mecum <[email protected]> Signed-off-by: Bryce Mecum <[email protected]>
…0022) ### What changes are included in this PR? This adds developer tooling to the repo for linting the docs by adding the sphinx-lint tool to archery and our pre-commit hooks. In both locations, only two rules are enabled at the moment (Discussed in apache#40006): `trailing-whitespace` and `missing-final-newline`. This PR also fixes the individual issues covered by the new tooling. ### Are these changes tested? Yes, though manually. I tested this works by running `archery lint --docs` and `pre-commit` without and without changes that should get caught by the rules. It works as expected. ### Are there any user-facing changes? Yes, 1. Developers that use pre-commit hooks will see a change in behavior when they modify docs 2. Developers using archery will see a new --docs option in `archery lint` 3. Developers working on the docs may see CI failures related to the new checks * Closes: apache#39990 * GitHub Issue: apache#39990 Authored-by: Bryce Mecum <[email protected]> Signed-off-by: Bryce Mecum <[email protected]>
…0022) ### What changes are included in this PR? This adds developer tooling to the repo for linting the docs by adding the sphinx-lint tool to archery and our pre-commit hooks. In both locations, only two rules are enabled at the moment (Discussed in apache#40006): `trailing-whitespace` and `missing-final-newline`. This PR also fixes the individual issues covered by the new tooling. ### Are these changes tested? Yes, though manually. I tested this works by running `archery lint --docs` and `pre-commit` without and without changes that should get caught by the rules. It works as expected. ### Are there any user-facing changes? Yes, 1. Developers that use pre-commit hooks will see a change in behavior when they modify docs 2. Developers using archery will see a new --docs option in `archery lint` 3. Developers working on the docs may see CI failures related to the new checks * Closes: apache#39990 * GitHub Issue: apache#39990 Authored-by: Bryce Mecum <[email protected]> Signed-off-by: Bryce Mecum <[email protected]>
Describe the enhancement requested
It would be good to look into what we currently have enabled and what we could easily add for linting docs PRs. #39989 found two types of errors, both of which would be good to avoid. I think sphinx-build already offers some form of reStructuredText linting and there appear to be other tools such as https://github.com/sphinx-contrib/sphinx-lint.
Component(s)
Continuous Integration, Documentation
The text was updated successfully, but these errors were encountered: