-
Notifications
You must be signed in to change notification settings - Fork 73
Partial configuration #811
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
base: master
Are you sure you want to change the base?
Conversation
|
CI is red. Also, this PR should change docs: docs/content/how-tos/rule-configuration.md (see the part added in PR489) |
e88ca5f to
8649a49
Compare
|
Let's add one last commit to this PR: a config file that will just contain the typePrefixing rule with mode=Strict. This will be in a directory where SelfCheck runs. The rule will of course also removed from excludedRules var in this same commit. |
Sorry, mode=Always |
Wrong order of commits, test should be added first before making it pass. |
dc9cc04 to
c2b24f4
Compare
Reordered the commits. Put tests first. |
Given this commit, then I guess PR title needs to be updated. |
Implemented partial configs and added tests for combining configs.
Added command line option "--partial-config" that when used, treats config file specified using "--lint-config" as partial config (contains only configs that should be overriden; rules not specified in it use default values).
Co-authored-by: webwarrior-ws <reg@webwarrior.ws>
And remove --partial-config flag. Configs will now be always partial.
By using camelCase rule name name in config instead of PascalCase (camelCase is proper rule naming scheme in fsharplint.json).
To reflect change in how config is applied.
Co-authored-by: webwarrior-ws <reg@webwarrior.ws>
da89ff4 to
33f608d
Compare
When combining configs, if both base and overriding configs have Hints config, combine `add` and `ignore` arrays in HintConfig objects instead of totally overriding base hint config by overriding one. Also actually use `ignore` array to filter out available hints when flattening config. Before, `ignore` was not used at all. Fixes fsprojects#466
18c029c to
5360bb6
Compare
90bb50d to
d3cfd45
Compare
With Mode=Always.
d3cfd45 to
c470525
Compare
| Configuration.combineConfigs defaultConfig partialConfig |> Ok | ||
| | Error(_) as err -> err | ||
| with | ||
| | ex -> Error (string ex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@webwarrior-ws why are you swallowing the exception here? (BTW, is string ex the same as ex.ToString()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@webwarrior-ws why are you swallowing the exception here? (BTW, is
string exthe same asex.ToString()?
That is what original code was doing (catching exception and putting it into Error). I assume it was done because Configuration.loadConfig can throw an exception but getConfig returns Result, and so is not expected to throw.
string ex is the same as ex.ToString().
Treat config file specified using "--lint-config" as partial config (contains only configs that should be overridden; rules not specified in it use default values).
Supersedes #711
Closes #488