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.

Sign up for GitHub

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] add yaml files linter to pre-commit hook but skip some rules for now (Part 1) #6763

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+19 −0
Open

[ci] add yaml files linter to pre-commit hook but skip some rules for now (Part 1) #6763

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
643a5b9
Update .pre-commit-config.yaml
StrikerRUS Dec 18, 2024
5243e55
Create .yamllint.yml
StrikerRUS Dec 18, 2024
7ec64bc
Update static_analysis.yml
StrikerRUS Dec 18, 2024
3a61605
Update lint-python-bash.sh
StrikerRUS Dec 18, 2024
14051cf
Update static_analysis.yml
StrikerRUS Dec 18, 2024
4adf207
disable some rules instead of skipping yamllint hook
StrikerRUS Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ci/lint-python-bash.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
set -e -E -u -o pipefail

echo "running pre-commit checks"
pre-commit run --all-files || exit 1
SKIP=yamllint pre-commit run --all-files || exit 1
echo "done running pre-commit checks"

echo "running mypy"
Expand Down
5 changes: 5 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ repos:
hooks:
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://github.com/adrienverge/yamllint
rev: v1.35.1
hooks:
- id: yamllint
args: ["--strict"]
Copy link
Collaborator

@jameslamb jameslamb Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This will be a bit disruptive to local development.. running pre-commit run --all-files locally (which I do many times during local development)... will produce a lot of output and fail until we've fixed all the issues.

Would you consider the following instead?

  • add all the rules that currently produce errors to .yamllint.yml in this PR with :disable
  • remove the SKIP=yamllint in lint-python-bash.sh
  • enable more and more rules over each of the next set of PRs.

That would prevent any new issues from being introduced on master while allowing pre-commit run --all-files to continue working locally.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense! Thank you very much for the suggestion! I just pushed 4adf207 with these changes.

- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.8.3
Expand Down
10 changes: 10 additions & 0 deletions .yamllint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# default config: https://yamllint.readthedocs.io/en/stable/configuration.html#default-configuration
extends: default

rules:
document-start: disable
line-length:
max: 120
truthy:
# prevent treating GitHub Workflow "on" key as boolean value
check-keys: false
Loading