Improve ESLint config

[maxpatiiuk]

• [ ] Look into https://github.com/mizdra/eslint-interactive
• [ ] run analysis on whole codebase to manually fix easy issues
• [ ] Fix all ESLint issues
• [ ] Adjust ESLint rules to disable/soften counter-productive rules
• [ ] Evaluate all eslint-disable-next-line comments
• [ ] Run Problems analyzer in PyCharm to detect issues (it can detect some things that ESLint doesn't look at)
• [ ] Use https://www.npmjs.com/package/eslint-restricted-globals to prevent bugs (i.e, when variable called name or length is used in the code without declaration, no error is thrown because those are available in global scope)

• [ ] Improve the GitHub Action (https://github.com/specify/specify7/blob/production/.github/workflows/test.yml)

  • [ ] Use lint-staged

  • [ ] Use husky pre-commit hooks

  • [ ] Error on non-auto-fixable eslint errors

  • [ ] Swap committer and author (I confused the two)

  • [ ] Either get rid of dorny/paths-filter, or pass correct base to it. base: ${{ github.ref }} appears to be wrong as seen by it using origin/production here for commit that is for the origin/xml-editor branch: (though maybe that was because github opens PRs for the production branch by default)

  • FYI: can see github action context like this:

    - name: Dump GitHub context
    env:
      GITHUB_CONTEXT: ${{ toJson(github) }}
    run: echo "$GITHUB_CONTEXT"

[melton-jason]

in our team at Esri, they found more luck in encouraging people to write tests than enforcing people to use certain code style (though tests are still not always written) because people are too opinionated about the code style OR don't care enough about using consistent code style (some people care about shipping features more than having code look a certain way - both sides have their merits).

my personal opinion is that writing code is more fun than writing tests, so if you can use strict ESLint config and strict typing (with TypeScript) to reduce the need for writing tests (while also enforcing higher-quality code), then that's great.

plus, having TypeScript and ESLint catch and report issues before PR is created is great as it reduces PR review time. not to mention many benefits of consistency in the codebase

I would like to spend some time in the coming months improving our ESLint config, after 7.9.4 is out, I would go about fixing all remaining ESLint errors, and enabling 0 ESLint error enforcement in Github action. Important context about this:

• I would disable/loosen ESLint rules that in the past 2 years have been more of a hindrance/annoyance/noise than actual help

  • example of noise is when we have a linting rule discouraging creating classes, having a linting rule discouraging the use of this is mostly redundant
  • some rules are designed to prevent bugs, that aren't likely to happen thanks to typescript (i.e the rule requiring that each .then() returns something), thus are redundant/hindrance
  • while I liked the "writing-good-comments" eslint rules in theory, they weren't too helpful in practice. I.e resolving the rule about "no passive voice in comments" often required making comments more verbose (or we just ignored the errors suggested by this rule)
• I would look for more useful rules to enable
• I would ask for more input and feedback before imposing the rules on others

@melton-jason @CarolineDenis @realVinayak thoughts about all of this?

From https://github.com/specify/specify7/pull/4273#discussion_r1427485477 by @maxpatiiuk