"Unchanged files with check annotations" flags distracting lint errors

[jgravois]

Describe the bug

i'm seeing lots of distracting annotations to unchanged lines of code in recent open PRs in this project.

As of the time of writing, it doesn't appear advanced configuration options exist which would allow you to flag only new warnings (ref: actions/toolkit#457).

I see 769 warnings when running npm run lint locally, with pros/cons to several plausible solutions.

1. ignore the noise indefinitely (🙉)
2. fix the existing warnings (tedious 📚)
3. disable the pertinent rules altogether (a bit too nuclear 💣)

// .eslintrc.js
"rules": {
  "indent": "off",
  "comma-dangle": "off",
  "@typescript-eslint/indent": "off"
}

To Reproduce
This Github feature is flagged as Beta so its entirely possible that it is being deployed selectively and others aren't seeing it, but in my experience new features are usually all or nothing for an organization. 🤷

Expected behavior
Github Actions doesn't flag any stable code.

Severity
Not severe at all, just an annoyance/distraction to contributors, maintainers and voyeurs alike.

I'm happy to submit a patch myself if fixing the existing warnings is actually what y'all want to see happen.

[antony]

These warnings are a result of the new + updated eslint configuration. There were a number of new rules from eslint, which we want to enforce, but at the same time, don't want to cause existing PRs to be unmergable.

So right now they show as warnings, which I'll concede, is quite annoying.

Ideally we'd fix these errors (it's not tedious, it's just npm run lint -- --fix). However I'm unsure of the impact on existing PRs, which is a concern.

[jgravois]

thanks for taking the time to lay out the backstory.

for me locally, --fix is sufficient to remove the trailing commas, but the 266 indentation errors in the .ts files remain, regardless of how many times the command is run.

i did a quick search and found typescript-eslint/typescript-eslint#1824 indicating that the typescript-eslint indent "rule is in a broken state". 😬

at the same time, don't want to cause existing PRs to be unmergable.

with a project attracting this many contributions, i'm not sure you're ever going to find a perfect opportunity to rip off the band-aid. if these are lint rules you want to enforce going forward it doesn't seem like a terrible idea to me to just get it over with and let contributors resolve any merge conflicts that arise.

like i said before though, its just a minor annoyance. if you'd prefer to just let sleeping dogs lie, that's a-okay too.

[antony]

like i said before though, its just a minor annoyance. if you'd prefer to just let sleeping dogs lie, that's a-okay too.

Believe you me I'd have the thing tied down to the letter by now. It was my betters that suggested a softly-softly approach ;)

We'll start by fixing the warnings (with agreement from others) and then see how it affects the existing PRs. Happy to take a PR to clear up the warnings if you're in such a position.

[jgravois]

with regard to the indentation specifically, i think the correct course of action is to:

1. turn off @typescript-eslint/indent
2. have indent ignore .ts files (not sure if you can even do that)

i'll make time soon to investigate.