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

Call ApplyConfig before ValidateRules #1043

Conversation

richardTowers
Copy link
Contributor

I'm working on a plugin which generates a list of rules dynamically,
based on its config:

https://github.com/richardTowers/tflint-ruleset-workspaces/

At the moment, it's not possible to refer to config in RuleNames because
that gets called before ApplyConfig. As a workaround, I've made my
plugin return the empty list for RuleNames:

https://github.com/richardTowers/tflint-ruleset-workspaces/blob/master/ruleset.go#L28

Naïvely, it looks like making the ApplyConfig a bit earlier would fix
this.

The tests pass, but I'm not sure if this will have unintended
consequences. Careful review would be much appreciated!

I'm working on a plugin which generates a list of rules dynamically,
based on its config:

https://github.com/richardTowers/tflint-ruleset-workspaces/

At the moment, it's not possible to refer to config in RuleNames because
that gets called before ApplyConfig. As a workaround, I've made my
plugin return the empty list for RuleNames:

https://github.com/richardTowers/tflint-ruleset-workspaces/blob/master/ruleset.go#L28

Naïvely, it looks like making the ApplyConfig a bit earlier would fix
this.

The tests pass, but I'm not sure if this will have unintended
consequences. Careful review would be much appreciated!
Copy link
Member

@wata727 wata727 left a comment

Choose a reason for hiding this comment

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

Hmm, originally, I didn't expect to generate rules dynamically, but I don't think this change will cause any major problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants