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

feat: Add nested dictionary formating check #284

Merged
merged 1 commit into from
Jul 23, 2022

Conversation

sblaisot
Copy link
Contributor

This pull request add a check on proper nested dictionary over-indentation as advised in https://docs.saltproject.io/en/latest/topics/troubleshooting/yaml_idiosyncrasies.html#nested-dictionaries

It adds Rule 219 (which seems to be the first never-used 2xx rule number). Feel free to change rule number if needed.

This fixes #282

@sblaisot sblaisot force-pushed the nested-dict-indent branch 2 times, most recently from 913df48 to 0d84ca2 Compare July 19, 2022 16:21
@sblaisot sblaisot force-pushed the nested-dict-indent branch 2 times, most recently from cacab91 to 312f009 Compare July 19, 2022 16:43
@jbouter
Copy link
Member

jbouter commented Jul 19, 2022

Hi again @sblaisot!

Thanks so much for your PR. Looks like you ticked all the right boxes on your PR as well! We'll try to do a code review as soon as we can, and integrate this new feature into our next release if it passes :-)

Copy link
Contributor

@matthijswendelaar matthijswendelaar left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @sblaisot! The PR looks good overall, I have made a few suggestions to avoid some false-positives and have correct versioning of the rule.

saltlint/rules/NestedDictRule.py Outdated Show resolved Hide resolved
saltlint/rules/NestedDictRule.py Outdated Show resolved Hide resolved
tests/unit/TestNestedDictRule.py Show resolved Hide resolved
@sblaisot sblaisot force-pushed the nested-dict-indent branch from 312f009 to 1a2a422 Compare July 22, 2022 19:01
@sblaisot
Copy link
Contributor Author

Thanks for review @matthijswendelaar
All comments addressed

Copy link
Contributor

@matthijswendelaar matthijswendelaar left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @sblaisot, I made a single suggestion to revert back to your original of 2 spaces of over-indentation required, instead of only 1 space of over-indentation required that I suggested.

After that the PR can be merged.

saltlint/rules/NestedDictRule.py Outdated Show resolved Hide resolved
@sblaisot sblaisot force-pushed the nested-dict-indent branch from 1a2a422 to 5327c4e Compare July 23, 2022 08:20
@matthijswendelaar matthijswendelaar merged commit 42669e8 into warpnet:main Jul 23, 2022
@sblaisot sblaisot deleted the nested-dict-indent branch July 23, 2022 08:28
@roaldnefs roaldnefs added this to the Version 0.9.0 release milestone Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add check for proper nested dict indentation
4 participants