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

Update our yamllint configuration file #200

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

mcdonnnj
Copy link
Member

@mcdonnnj mcdonnnj commented Dec 4, 2024

🗣 Description

This pull request updates the .yamllint configuration file. It does the following:

  • Add settings for the braces rule.
  • Add settings for the brackets rule.
  • Add settings for the comments rule.
  • Add settings for the indentations rule.
  • Add settings for the octal-values rule.
  • Add settings for the quoted-strings rule.
  • Remove settings for the truthy rule.

💭 Motivation and context

These changes are a mix of matching the configuration to our existing best practices to ensure compliance and also updating the rules for changes in Ansible/ansible-lint.

Warning

The new octal-values rule settings are a breaking change from how we currently use octal values in Ansible (and probably elsewhere). We should probably determine if we prefer the "0xxx" or "0oxxx" form

🧪 Testing

Automated tests pass. This will need some massaging downstream in repos with a .yamllint based on the ansible-lint default, and those .yamllint files should be updated to reflect the current default configuration as described in the documentation.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

Use a specific number of spaces instead of the default of only caring
if the number of spaces used is consistent within a file. Ensure that
block sequences inside of mappings are indented.
The use of flow sequences and mappings is not as readable as block
collections and so should be discouraged. Since it is a cleaner
representation for empty collections we will allow those, but if an
application otherwise requires flow collections they can be explicitly
enabled by disabling the checks per
https://yamllint.readthedocs.io/en/stable/disable_with_comments.html
When running ansible-lint it will throw the following warning with our
current configuration:
WARNING  Found incompatible custom yamllint configuration (.yamllint), please either remove the file or edit it to comply with:
  - comments.min-spaces-from-content must be 1
  - braces.max-spaces-inside must be 1
  - octal-values.forbid-implicit-octal must be true
  - octal-values.forbid-explicit-octal must be true.
Thus we implement these configuration rules.
Previously we disabled the `truthy` rule due to Ansible's use of
`yes`/`no` for boolean values. That is no longer the case and the
default configuration used by ansible-lint now has this rule enabled.
The use of `on` as a key in GitHub Actions workflow syntax means we
needed to add disable-line comments for the truthy rule.
@mcdonnnj mcdonnnj added breaking change This issue or pull request involves changes to existing functionality improvement This issue or pull request will add or improve functionality, maintainability, or ease of use security This issue or pull request addresses a security issue labels Dec 4, 2024
@mcdonnnj mcdonnnj self-assigned this Dec 4, 2024
Add a configuration for the `quoted-strings` rule that matches our best
practices. Other files are updated to comply with these new settings.
@mcdonnnj mcdonnnj force-pushed the improvement/update_yamllint_rules branch from 6ed0ef0 to dc891af Compare December 4, 2024 21:27
.yamllint Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This issue or pull request involves changes to existing functionality improvement This issue or pull request will add or improve functionality, maintainability, or ease of use security This issue or pull request addresses a security issue
Projects
Status: Reviewer approved
Development

Successfully merging this pull request may close these issues.

3 participants