feat: strict mode for warnings #367
Conversation
|
Supersedes #366. This is a middle-ground that:
|
|
My perspective is this:
I feel like with this PR we're trying to implement a generic solution for a specific problem. Also, I don't really have an opinion about the broader discussion about warnings VS errors, so will refrain from commenting on that. |
Me too, so this PR doesn't change anything in terms of usage.
This PR only provides a way to see rules configured as warnings in the console. Today they are completely ignored with no way of seeing them at all. Maybe
I share this fear, and would like to deal with it elsewhere, so this PR keeps things as they are.
Nice examples of alternatives! Those could indeed be ways to go about it. Internally those options would require significant structural changes in how commands are organized, as well as how we configure and run the toolchains. |
|
In terms of what flags to parse to the command, I'd prefer more explicit, less ambiguous flags: With the above flags, we would have the current behavior (no flags), the "strict" mode ( |
I guess these would be more acceptable to @HendrikThePendric as well (no structural/conceptual changes)? |
|
This seems similar to an issue that was filed with eslint: eslint/eslint#2309 ( What I was proposing was basically to let eslint do its thing, without letting warnings fail anything, and without swallowing them. The way I understand the proposed strict mode here (which is similar to the proposed strict mode in the issue), is that it would exit with a non-zero exit code on warnings. As the maintainer in the issue there mentioned I also feel that that logic blurs the line between errors and warnings (a warning by definition shouldn't fail anything, then we should just make it an error). Basically what I meant is just for us to remove the edit: Allowing |
The way d2-style was designed was to be a gatekeeper. That was its reason for existing, it has no nuance. Either it is good or it is bad. What it is evolving to become is a developer support tool. And guidance requires more nuance than enforcing, so I see where the need for both errors and warnings in that sense.
The tools that d2-style used to accomplish its agenda were intended to be implementation details that are completely obscured from the user. This was changed when developers requested to be able to see lint/formatting errors in their editors.
My intent was for the user to use it manually to check if there are any "soon to be problems", not that it should be enabled anywhere. |
|
I need to finish up my implementation of the That would be my enforcer of good and bad. The other commands could be allowed more nuance to decide what is ugly. |
This removes the `--quiet` flag from the ESLint invocation, and fixes the broken output where ESLint complains about us trying to lint dot-files and folders by un-ignoring their built-in ignore filters. We agree with ignoring node_modules in terms of linting however, so that is left in place.
I see what you mean. Yeah it seems like we both had a slightly different perception of the design goals of cli-style, thanks for explaining! |
Allow d2-style to print warnings from ESLint and introduce a concept of
strict and non-strict modes.
Strict mode: Set a zero tolerance on warnings by making ESLint exit with
a non-zero code if there are any warnings. Warnings will be shown in
strict mode.
Non-strict mode: Allow ESLint to exit with code zero when there are
warnings. Warnings are essentially ignored in this mode which matches
the behavior today.
This allows gradual introduction of new rules, that may later become
errors.
In strict mode a developer is expected to either:
a) fix the warning
b) explicitly mark the code that triggered the warning with an
eslint-disable comment.
A good disable directive uses the comment syntax for ESLint:
E.g.
When rules graduate from "warning" to "error" the code base should be
ready for the change.