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

Add rule to check key order #2108

Merged
merged 12 commits into from May 12, 2022
Merged

Add rule to check key order #2108

merged 12 commits into from May 12, 2022

Conversation

jeefberkey
Copy link
Contributor

@jeefberkey jeefberkey commented May 8, 2022

Adds a new rule that for the moment checks that name is the first key on tasks. In the future
we will add additional ordering rules to it.

Partial: #578

@ssbarnea
Copy link
Member

@jeefberkey Please update PYTEST_REQPASS and increase its value to match expected number of tests (+2).

@jeefberkey
Copy link
Contributor Author

Thanks for the additional guidance - rg was ignoring the directory when I was looking for that number.

@ssbarnea
Copy link
Member

Thanks for the additional guidance - rg was ignoring the directory when I was looking for that number.

I am an rg user myself, great tool! I have an alias rg='rg --hidden --no-follow --max-columns 255 --no-heading --column -F' that you might like.

@ssbarnea
Copy link
Member

@jeefberkey I pushed some minor changes, like removing opt-int, we will make it experimental from start as it is very useful.

@ssbarnea ssbarnea added this to the 6.2.0 milestone May 10, 2022
@ssbarnea ssbarnea changed the title Add check for making sure name if the first attribute in task Add rule to check key order May 11, 2022
@jeefberkey
Copy link
Contributor Author

Thanks for the test fixes, I was working on them but was sick the past few days. The new test name may conflict with other things I'm working on, but we can deal with that later

Copy link
Contributor

@cidrblock cidrblock left a comment

Choose a reason for hiding this comment

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

love it.

@ssbarnea
Copy link
Member

Thanks for the test fixes, I was working on them but was sick the past few days. The new test name may conflict with other things I'm working on, but we can deal with that later

Please mention more details about conflicts we might have? While class name is not important, the id of the rule is harder to change once we merge it, so better to get it right from start. I used key-order because I want to evolve this rule to take care of ordering of keys in general.

@jeefberkey
Copy link
Contributor Author

Nothing, this is actually not a blocker now that I think about it.

@ssbarnea ssbarnea merged commit ad4cc87 into ansible:main May 12, 2022
@jeefberkey jeefberkey deleted the task-name-first branch May 12, 2022 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants