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

[Docs][CI] Enable more sphinx-lint rules for documentation #41611

Closed
amoeba opened this issue May 9, 2024 · 4 comments
Closed

[Docs][CI] Enable more sphinx-lint rules for documentation #41611

amoeba opened this issue May 9, 2024 · 4 comments

Comments

@amoeba
Copy link
Member

amoeba commented May 9, 2024

Describe the enhancement requested

The work in #39990 enabled sphinx-lint as a pre-commit hook but enabled just two rules so as to start off simple:

  • trailing-whitespace
  • missing-final-newline

sphinx-lint offers a total of 22 rules and some of these are clearly useful as, at least at the time of writing, they catch meaningful meaningful problems in our documentation sources.

We should consider the full ruleset and enabling as many as we think offer good value.

Component(s)

Continuous Integration, Documentation

@amoeba
Copy link
Member Author

amoeba commented May 9, 2024

The current docs violates just six of the 22 sphinx-lint rules. Of these, most are useful and catch real issues. For each, I've noted which I think we should and shouldn't enable:

  • dangling-hyphen
    • No. This is meant to catch hyphens breaking up long words for hard wrapping but we use dangling hyphens for some anchors and our use looks legit.
  • missing-space-after-literal
  • horizontal-tab
    • Yes. This seems like a good catch since tabs don't get treated correctly in HTML anyway and it makes it hard to know whether the source will render correctly.
  • missing-final-newline
    • Yes. A bit nitpicky but it makes things more regular which should avoid noise in diffs.
  • missing-underscore-after-hyperlink
  • trailing-whitespace
    • Yes. Again a bit nitpicky but it makes the docs more regular which avoids diff noise.
  • unbalanced-inline-literals-delimiters
    • Yes. This catches real issues where authors forget to close their literals.

I think we could get away with just disabling dangling-hyphen and enabling all other checks. I'll still go through the other checks to make sure we don't enable something we don't want but, since we don't violate most of the rules, we can disable any we find that don't add value in the future. I'll update here once I do that.

@AlenkaF
Copy link
Member

AlenkaF commented May 13, 2024

Thank you for working on this @amoeba and adding a thorough summary. Agree with what is added till now 👍

@amoeba
Copy link
Member Author

amoeba commented May 16, 2024

Thanks @AlenkaF. I marked the PR as ready for review and I tagged you for review in addition to @jorisvandenbossche since he saw the last PR.

AlenkaF pushed a commit that referenced this issue May 16, 2024
…41612)

### Rationale for this change

#41611

### What changes are included in this PR?

- Update to pre-commit config to enable all checks except `dangling-hyphen`, `line-too-long` by default
- Associated fix docs

### Are these changes tested?

Yes, by building and looking at the docs locally.

### Are there any user-facing changes?

Just docs.
* GitHub Issue: #41611

Authored-by: Bryce Mecum <petridish@gmail.com>
Signed-off-by: AlenkaF <frim.alenka@gmail.com>
@AlenkaF AlenkaF added this to the 17.0.0 milestone May 16, 2024
@AlenkaF
Copy link
Member

AlenkaF commented May 16, 2024

Issue resolved by pull request 41612
#41612

@AlenkaF AlenkaF closed this as completed May 16, 2024
vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
…ation (apache#41612)

### Rationale for this change

apache#41611

### What changes are included in this PR?

- Update to pre-commit config to enable all checks except `dangling-hyphen`, `line-too-long` by default
- Associated fix docs

### Are these changes tested?

Yes, by building and looking at the docs locally.

### Are there any user-facing changes?

Just docs.
* GitHub Issue: apache#41611

Authored-by: Bryce Mecum <petridish@gmail.com>
Signed-off-by: AlenkaF <frim.alenka@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants