[docs] Ray air precommit fix#59586
Conversation
## Context PR ray-project#57799 removed ci/lint/format.sh and the `code_format` job from the microcheck pipeline in favor of pre-commit-only checks. However, this created a gap in coverage: .pre-commit-config.yaml globally excludes doc/source/, meaning Python/shell files in documentation are no longer formatted or linted. ## Problem Many Anyscale console templates use content from doc/source/, including .py, .sh, and .yaml files that users reference. These files should follow the same formatting standards as the rest of the Ray repo. The old format.sh checked doc/source/ with: - black (Python formatting) - shellcheck (shell script linting) - docstyle (Ray docstring style) - check-import-order (import ordering) ## Solution Updated .pre-commit-config.yaml to restore formatting checks on doc/source/: 1. Changed global exclude from "doc/source/" to only exclude notebooks 2. Added "exclude: ^doc/source/" to 14 hooks that weren't in format.sh 3. Kept black, shellcheck, docstyle, and check-import-order enabled Also marked the dead code_format() function in ci/lint/lint.sh for removal. ## Files Changed - .pre-commit-config.yaml: Restore formatting checks for doc/source/ - ci/lint/lint.sh: Mark dead code_format() function for removal Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Signed-off-by: Aydin Abiar <aydin@anyscale.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to improve documentation quality by enabling vale linting and adjusting pre-commit configurations to exclude documentation files from irrelevant checks. The changes in .pre-commit-config.yaml and the documentation .rst files are positive improvements. However, I've found some issues in the vocabulary files that need addressing for consistency and maintainability. Additionally, there's an unexplained removal of a CI function in ci/lint/lint.sh which poses a potential risk to code quality and should be clarified.
I am having trouble creating individual review comments. Click here to see my feedback.
ci/lint/lint.sh (55-58)
The code_format function is being removed, but the pull request description doesn't explain this change. This function seems to run an important formatting script (./ci/lint/format.sh). Is this functionality now fully covered by the pre-commit hooks? Removing a CI check without clarification is risky as it could allow formatting inconsistencies to slip through. Please provide a reason for this removal or restore the function if its checks are not redundant.
.vale/styles/config/vocabularies/General/accept.txt (53)
This change introduces a duplicate entry. [Aa]sync is already present on line 3. Please remove this duplicate.
I've also noticed other new duplicates/redundancies in this file introduced by this PR:
Istioon line 194 is redundant with(?i)Istioon line 239. The case-insensitive version is preferable.
While you're editing this file, you might also want to remove the pre-existing duplicate deserialization on lines 66-67 for better maintainability.
.vale/styles/config/vocabularies/Serve/accept.txt (2)
The comment on this line states that new terms should be added alphabetically, but the list below is not sorted. For example, Arize appears after [Uu]pscales. Please sort the list to follow the established convention and improve maintainability.
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
github-actions
bot
added
the
stale
|
This pull request has been automatically closed because there has been no more activity in the 14 days Please feel free to reopen or open a new pull request if you'd still like this to be addressed. Again, you can always ask for help on our discussion forum or Ray's public slack channel. Thanks again for your contribution! |
1 participant
doc/source has been excluded from pre-commit, fixing current format/lint errors before including it back.