Allow rules string as a configuration alternative #19
Conversation
filipesperandio
commented
Sep 25, 2017
•
filipesperandio
commented
filipesperandio
force-pushed
the
fe/rules2
branch
from
8120b86 to
7a2eda6
Compare
fixtures/rules/config.json
Outdated
| "enabled": true, | ||
| "channel": "beta", | ||
| "config": { | ||
| "rules": "java-basic,java-design" |
There was a problem hiding this comment.
Is there a reason why this isn't an array (instead of a string with a comma-separated list)?
There was a problem hiding this comment.
Because I am stupid I guess... =/
There was a problem hiding this comment.
Ha, no way! 😛
Just a suggestion to simplify parsing. 😄
There was a problem hiding this comment.
Actually we just forward the entire thing, I think that was the reason I didn't bother to split, but I makes more sense from a yml perspective to have it as an array.
There was a problem hiding this comment.
@toddmazierski Adjusted.
| @Test | ||
| public void acceptRulesSimpleNames() { | ||
| def config = new Config([configFile: "/usr/src/app/fixtures/rules/config.json", codeFolder: "/usr/src/app/fixtures/rules"]) | ||
| assertEquals "java-basic,java-design", config.ruleSet() |
There was a problem hiding this comment.
Should this assertion be adjusted?
There was a problem hiding this comment.
That's the argument for the command line, should still be a single string.
There was a problem hiding this comment.
Oh, my mistake.
Should config.mix.json still contain a comma-separated string, though?
There was a problem hiding this comment.
Does not impact, but it should be adjusted. 👍
