New: Control of minimumSeverity #723
Conversation
src/lint/lint.ts
Outdated
| @@ -45,10 +45,9 @@ export async function lint(options: Options, config: ProjectConfig) { | |||
| const rules = lintLib.registry.getRules(ruleCodes || lintOptions.rules); | |||
| const filter = new WarningFilter({ | |||
| warningCodesToIgnore: new Set(lintOptions.ignoreWarnings || []), | |||
| minimumSeverity: Severity.WARNING | |||
| minimumSeverity: options.minSeverity || lintOptions.minSeverity || Severity.WARNING, | |||
There was a problem hiding this comment.
Would still need to add minSeverity to the polymer-project-config lint section.
Side Note: It seems that trailing comma is mixed in this repo.
There was a problem hiding this comment.
This doesn't compile because project config doesn't have minSeverity. Options:
- don't include the reference
- reference it using a string literal
lintOptions['minSeverity']
In either case we should have a TODO here to add it to project config
CHANGELOG.md
Outdated
| @@ -2,6 +2,7 @@ | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| - **Updated `lint` Command**: `polymer lint` now has finer control over what is output and causes the command to fail. | |||
There was a problem hiding this comment.
Please be more explicit here. Mention the new flag and how to use it.
src/lint/lint.ts
Outdated
| @@ -45,10 +45,9 @@ export async function lint(options: Options, config: ProjectConfig) { | |||
| const rules = lintLib.registry.getRules(ruleCodes || lintOptions.rules); | |||
| const filter = new WarningFilter({ | |||
| warningCodesToIgnore: new Set(lintOptions.ignoreWarnings || []), | |||
| minimumSeverity: Severity.WARNING | |||
| minimumSeverity: options.minSeverity || lintOptions.minSeverity || Severity.WARNING, | |||
There was a problem hiding this comment.
This doesn't compile because project config doesn't have minSeverity. Options:
- don't include the reference
- reference it using a string literal
lintOptions['minSeverity']
In either case we should have a TODO here to add it to project config
src/lint/lint.ts
Outdated
| @@ -45,10 +45,9 @@ export async function lint(options: Options, config: ProjectConfig) { | |||
| const rules = lintLib.registry.getRules(ruleCodes || lintOptions.rules); | |||
| const filter = new WarningFilter({ | |||
| warningCodesToIgnore: new Set(lintOptions.ignoreWarnings || []), | |||
| minimumSeverity: Severity.WARNING | |||
| minimumSeverity: options.minSeverity || lintOptions.minSeverity || Severity.WARNING, | |||
There was a problem hiding this comment.
Hm, warning severity should really be a string enum rather than the numeric enum I used -.-
I guess there's still time for a breaking change there if we're very quick.
In any case, for now, you'll need to convert the string to the enum value, something like:
function severityStringToValue(maybeSeverity: string): Severity|undefined {
maybeSeverity = maybeSeverity.trim().toLowerCase();
switch (maybeSeverity) {
case 'error': return Severity.ERROR;
case 'warning': return Severity.WARNING;
case 'info': return Severity.INFO;
default: return undefined;
}
}Then check for undefined and throw a useful error in before constructing the WarningFilter.
There was a problem hiding this comment.
Up to you if you would like to switch it or I can take a stab at it.
Also, it seems the analyzer has an inverse of this Severity to String. Maybe it would make sense to expose those in the Warning model so this doesn't get duplicated somewhere else? Just a thought.
I will add this for now.
|
There is a bit of conversation going on in Polymer/polymer-project-config#20 in case anyone wants to follow. |
|
Lets keep the discussion in Polymer/polymer-project-config#20 since it's already started there. Closing this PR for now, to be re-opened based on what we decide in Polymer/polymer-project-config#20 |
I really think it would be good to add a way to differentiate erroring and logging.
Took a shot at allowing control over minimumSeverity. Will replace #709 to fix #699.
@FredKSchott @rictic PTAL :)