Rules definition #41
Rules definition #41
Conversation
There was a problem hiding this comment.
Is it possible to write tests to ensure we don't break anything with all these new options?
There was a problem hiding this comment.
I agree.
What is the best way ?
Have analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), a, "somedir") for all 10 options with a different value than the default one ?
There was a problem hiding this comment.
The tests caught an error with fmt.Errorf("toto") becoming just a string when disabling error-format ;-)
There was a problem hiding this comment.
I'm less present lately for personal reasons.
I might reply with delay if you have questions, but here are my remarks.
And I agree with @alexandear tests are needed
As consistency between options is done at init time
and invert the negation in consequences
| r.Flags.BoolVar(&n.fiximports, "fiximports", true, "fix needed imports from other fixes") | ||
| r.Flags.BoolVar(&n.strconcat, "strconcat", true, "optimizes into strings concatenation") | ||
|
|
There was a problem hiding this comment.
You should not need all of that. Your algorithm shall start by checking if a format is activated and then trigger check their option.
Let's try to follow KISS principle here
There was a problem hiding this comment.
But some default options being true, I still need to do the check
|
I pushed a new version in #42 |
For #39
@alexandear @mmorel-35 @ccoVeille
What do you think of this ?