Skip to content

Commit

Permalink
Fix typos
Browse files Browse the repository at this point in the history
  • Loading branch information
softius committed Apr 22, 2024
1 parent a99c456 commit f9b3976
Showing 1 changed file with 13 additions and 12 deletions.
25 changes: 13 additions & 12 deletions designs/2024-baseline-support/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Declare currently reported errors as the "baseline", so that they are not being
<!-- Why are we doing this? What use cases does it support? What is the expected
outcome? -->

Enabling a new lint rule as `error` can be painful when the codebase has many violations and the rule isn't auto-fixable. A good example, is `no-explicit-any`. Unless the rule is enabled at the early stages of the project, is becoming harder and harder to enable it as the codebase grows. Existing violations must be resolved before enabling the rule, but while doing that other violations might creep in.
Enabling a new lint rule as `error` can be painful when the codebase has many violations and the rule isn't auto-fixable. A good example, is `no-explicit-any`. Unless the rule is enabled during the early stages of the project, it becomes harder and harder to enable it as the codebase grows. Existing violations must be resolved before enabling the rule, but while doing that other violations might creep in.

This can be counterintuitive for enabling new rules as `error`, since the developers need to address the violations before-hand in one way or another. The suggested solution suppress existing violations, allowing the developers to address these at their own pace. It also reports any new violations making it easier to identify and address future violations.

Expand All @@ -31,9 +31,9 @@ This can be counterintuitive for enabling new rules as `error`, since the develo
used. Be sure to define any new terms in this section.
-->

The suggested solution introduces the concept of a baseline file; A JSON file containing the number of errors that must be ignored for each rule in each file. By design, the baseline is disabled by default and it doesn't affect existing or new projects, unless the baseline file is generated.
To keep track of the all the errors that we would like to ignore, we are introducing the concept of the baseline file; A JSON file containing the number of errors that must be ignored for each rule in each file. By design, the baseline is disabled by default and it doesn't affect existing or new projects, unless the baseline file is generated.

Here is how the baseline file looks like. This indicates that the file `"src/app/components/foobar/foobar.component.ts"` has one error for the rule `@typescript-eslint/no-explicit-any` that is acceptable to be ignored.
Here is what the baseline file looks like. This indicates that the file `"src/app/components/foobar/foobar.component.ts"` has one error for the rule `@typescript-eslint/no-explicit-any` that is acceptable to be ignored.

```
{
Expand All @@ -51,13 +51,15 @@ A new option `--generate-baseline` can be added to ESLint CLI. When provided, th
eslint --generate-baseline ./src
```

The above will go through each result item and rules, and count the number of errors ( `severity == 2` ). If one or more such errors are found, the necessary details must be stored in the baseline file. The process should take place right after the fixes are applied to avoid counting errors that are about to be fixed.
The above will go through each result item and messages, and count the number of errors (`severity == 2`). If one or more such messages are found, the necessary details must be stored in the baseline file. The process should take place right after the fixes are applied to avoid counting errors that are about to be fixed.

### Matching against the baseline

The suggested solution always compares against the baseline, given that the baseline file `.eslint-baseline.json` exists. This makes it easier for existing and new projects to adopt the baseline without the need to adjust scripts in `package.json` and CI/CD workflows.

This will go through each result item and rules, and check each rule against the baseline. If the file and rule are part of the baseline, we decrease the number of errors to be ignored (in memory), and remove the result item. This needs to take place after the fixes are applied so that we compare the non-fixable issues only. It also needs to take place before the error counting, so that the remaining errors are counted correctly.
This will go through each result item and message, and check each rule giving an error (`severity == 2`) against the baseline. If the file and rule are part of the baseline, means that we can remove and ignore the result message. This needs to take place after the fixes are applied so that we compare the non-fixable issues only. It also needs to take place before the error counting, so that the remaining errors are counted correctly.

We can also keep track of which errors from baseline were not matched, that is useful for the next section.

### Maintaining a lean baseline

Expand All @@ -69,7 +71,7 @@ Caching must contain the full list of detected errors, even those matched agains

- Generating the baseline can be based on the cache file and should be faster when the cache file is used.
- Allows developers to re-generate the baseline or even adjust it manually and re-lint still taking the same cache into consideration.
- It even allows developers to delete completely the baseline and still take advantage of the cached file in subsequent runs.
- It even allows developers to delete the baseline and still take advantage of the cached file in subsequent runs.

## Documentation

Expand Down Expand Up @@ -107,7 +109,7 @@ If the baseline file is not generated, ESLint CLI behavior will not change. This

If the baseline file is generated, ESLint CLI will compare the errors against the errors included in the baseline. Hence it might report less errors than before and someone might argue that this is not backwards compatible since the behavior changes for them. However, as discussed earlier this should facilitate the adoption of the baseline without worrying about adjusting scripts in `package.json` and CI/CD workflow. Plus, the baseline can be easily deleted and cancel the new behavior.

Furthermore, we are adding one more reason to exit with an error code (see "Maintaining a lean baseline"). This might be have some negative side-effects to wrapper scripts that assume that error messages are available when that happens. We could introduce a different exit code, to differentiate between exiting due to unresolved errors or ignored errors that do not occur anymore.
Furthermore, we are adding one more reason to exit with an error code (see "Maintaining a lean baseline"). This might have some negative side-effects to wrapper scripts that assume that error messages are available when that happens. We could introduce a different exit code, to differentiate between exiting due to unresolved errors or ignored errors that do not occur anymore.

## Alternatives

Expand All @@ -134,11 +136,11 @@ Also, users can too easily ignore the new errors, so in a way, the rule is enabl

One can use disable comments to temporarily suppress errors, by adding a comment like `// eslint-disable rule-name -- FIX THIS LATER`

Disable comments can be used to enable a rule as an error early, by adding them everywhere where an error is currently reported (and that is actually something that can be automated by some linters).
"Disable comments" can be used to enable a rule as an error early, by adding them everywhere where an error is currently reported (and that is actually something that can be automated by some linters).

But disable comments have the tendency to be hard to distinguish from other disable comments created for reasons such as false positives or disagreements on the rule, especially when there is no enforced need to add a message on the comment. Meaning that once you decide to tackle the existing errors, they can be hard to detect (or to distinguish from ones that are disabled for good reasons).
But "disable comments" have the tendency to be hard to distinguish from other "disable comments" created for reasons such as false positives or disagreements on the rule, especially when there is no enforced need to add a message on the comment. Meaning that once you decide to tackle the existing errors, they can be hard to detect (or to distinguish from ones that are disabled for good reasons).

They also "pollute" the codebase in a way that is quite visible, and makes users numb to the fact of using disable comments.
They also "pollute" the codebase in a way that is quite visible, and makes users numb to the fact of using "disable comments".

### Ignoring parts of the project

Expand All @@ -147,8 +149,7 @@ It is also possible to simply disable the rule in each file that is currently re
This has multiple downsides:

* While new errors are enforced in the other files, new errors can creep in the ignored files
* If/when the errors in the ignored files get removed, the user has to remember to re-enable the rule on this file. Otherwise new errors can creep in also.

* If/when the errors in the ignored files get removed, the user has to remember to re-enable the rule on this file. Otherwise new errors can creep in.

## Open Questions

Expand Down

0 comments on commit f9b3976

Please sign in to comment.