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

Custom rules for verifying defaults against enums #43

Merged
merged 8 commits into from
Oct 22, 2024

Conversation

hawkeyexl
Copy link
Contributor

@hawkeyexl hawkeyexl commented Oct 2, 2024

Originated from the Write the Docs slack.

@lornajane
Copy link
Contributor

When reviewing, note that the code sample example is really similar to the changes I just merged in from #42 (the PRs were open at the same time)

@hawkeyexl
Copy link
Contributor Author

Please additionally note that they are similar because the code from #42 was in part based on the code present in this PR. The rules between the PRs are similar but distinct and complimentary.

@@ -0,0 +1,91 @@
# Require Bash Sample
Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/Redocly/redocly-cli-cookbook/pull/42/files#r1809495936
I left this comment on another similar PR. This doesn't feel so practical. Most people don't have bash samples and use curl for example (so "bash" is probably a rare/niche example for Redoc users).

I think it might be nice since this PR contains two separate plugins to separate it into two PRs, because the default enum one would be easy to approve.

This one should should probably be modified. I think the plugin would be more useful if it accepted an array of languages in the configuration and gave an error or warning (depending on severity) based on the list matching.

This is also similar to #42 , and I think if this was made more flexible should replace it. That one has a lot of quirks and hardcodes errors and warnings instead of taking the severity from the configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamaltman For now, I removed the bash example rule entirely. When I get a chance, I'll see if I can whip something together that's better suited, but I don't want to block the default/enum rule from getting merged.

@hawkeyexl hawkeyexl changed the title Custom rules for verifying defaults against enums and requiring a bash sample Custom rules for verifying defaults against enums Oct 21, 2024
@hawkeyexl
Copy link
Contributor Author

Removed require-bash-sample rule.

@hawkeyexl hawkeyexl requested a review from adamaltman October 21, 2024 21:58
@hawkeyexl hawkeyexl requested a review from adamaltman October 21, 2024 22:35
Copy link
Member

@adamaltman adamaltman left a comment

Choose a reason for hiding this comment

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

🎉 Thank you @hawkeyexl !

@adamaltman adamaltman merged commit 09608b7 into Redocly:main Oct 22, 2024
2 checks passed
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.

3 participants