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 key-order check to check all keys #2222

Closed
wants to merge 6 commits into from

Conversation

jeefberkey
Copy link
Contributor

A user can set the key_order setting to one of the following options:

  • default - checks only the order of name and action
  • enhanced - checks a smaller list of possible keys
  • everything - checks a list containing every key

@jeefberkey jeefberkey mentioned this pull request Jun 23, 2022
@jeefberkey
Copy link
Contributor Author

Will update this soon - struggling to find time to debug the typing errors recently

@ssbarnea
Copy link
Member

@jeefberkey Any chance you can resume work on this change? It was a nice addition to make to the linter.

A user can set the key_order setting to one of the following options:

* default - checks only the order of name and action
* enhanced - checks a smaller list of possible keys
* everything - checks a list containing every key
@jeefberkey jeefberkey marked this pull request as ready for review September 17, 2022 15:15
@jeefberkey jeefberkey requested review from a team as code owners September 17, 2022 15:15
@jeefberkey
Copy link
Contributor Author

@ssbarnea I picked this back up a few days ago - but my tests keep on failing for things I have not modified. Can you help me with that?

Also, it seems like the eco test have changed. Didn't the results used to be checked in to git? It looks like the test still uses git diff, so I'm confused.

@ssbarnea ssbarnea added this to the 6.6.0 milestone Sep 18, 2022
@ssbarnea ssbarnea self-assigned this Sep 18, 2022
@jeefberkey
Copy link
Contributor Author

Everything seems well now - I looked for a bit for a comprehensive list of allowed keys, but never found one. Hopefully the yaml file makes it a little easier to maintain.

@ssbarnea Thanks!

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

2 participants