Standardize linting across codebase

[ephemeralrogue]

There are a ton of installed linting packages and a slew of configuration files strewn throughout the codebase with inconsistent file types. A number of these packages can be removed in favor of utilizing a select few that will generally keep code restricted to agreed-upon conventions.

[pythonmcpi]

I'll list out the current tools, their config files, and what each tool does.

git: .gitattributes, .gitignore - Version control typescript: tsconfig.config.json, tsconfig.dev.json, tsconfig.json, tsconfig.settings.json: Typescript compiler prettier: .prettierrc - Code autoformatter

eslint: .eslintignore, .eslintrc.js - Linting tool for javascript @typescript-eslint - Extenstion for eslint to support typescript commitzen: .czrc - Interactive git commit that makes you fill out required fields commitlint: commitlint.config.js - Enforce commit message structures husky - Commit linting markdownlint: .markdownlint.yml, .markdownlintignore - Linter for markdown files

jest: jest.config.dev.js, jest.config.js - A javascript testing framework

typedoc: typedoc.json: Typescript documentation generator editorconfig: .editorconfig - Config format supported by a variety of code editors/ides

Semantic Release: .releaserc - Automated package releases

[ephemeralrogue]

I'll list out the current tools, their config files, and what each tool does.

Thank you, @pythonmcpi! This is helpful. A lot of this can be removed. The Commitzen comment seems nice, but that entails a lot more work with onboarding and adding an additional layer of effort on the shoulders of potential contributors; same with husky. We can scrap release automation for the time being and focus on developer workflows and shipping a usable package.

Regarding the config files, we can consolidate the file type and standardize configuration across the entire codebase. Reviewing the .eslintrc.js, there are very specific rules for a very specific style that we don't have to adhere too. Again, this comes with onboarding and contributor overhead. I'm in favor of rewriting .js and .yml config files to .json. In this process, we can scrap .eslintrc.js and create a limited set of constraints when creating the .eslintrc.json file.

I'm interested to hear your input, however, if you have other opinions.

[pythonmcpi]

I haven't looked at the contents of the config files much yet. In my past projects, I've used eslint (and @typescript-eslint) with the eslint:recommended and plugin:@typescript-eslint/recommended rules with no issues.

For my prettier config, I have semicolons (semi) set to always. I find it easier to always have semicolons rather than having to identify situations where automatic semicolon insertion behaves differently from what would be expected. However, the .prettierrc in this project has semicolons explicitly set to "only when required". The other prettier settings I care less about.

I'm in favor of dropping commitzen, husky, and sematic release (missed this one earlier, but edited it into the original comment). I don't use automation much, and would prefer to make this project as easy to contribute to as possible.

Here's my prettier config, compared with the project's current one

(d) means this is prettier's default setting

	My Config	Current Gauntlet Config	Comments
printWidth	200	80 (d)	I sometimes have long lines in my code that I don't want to wrap. I'm fine if we choose to use 80 columns for this project.
tabWidth	4	2	I find it easier to read code when it's indented more.
singleQuote	false (d)	true
semi	true (d)	false	See the 2nd paragraph
bracketSpacing	true (d)	false	I find it easier to read
arrowParens	always (d)	avoid	I'll summarize prettier's documentation for why their default is always: it's easier to add type annotations, extra arguments, and default values, and provides more consistency when editing codebases.
trailingComma	es5 (d in v2)	none	Trailing commas reduce the number of diff lines when adding entries to arrays or objects. Prettier 2 has "es5" as the default option, but this was changed in Prettier 3 to be "all" (so trailing commas are added to function parameters and calls). We shouldn't have issues switching this to "all", as we target Node.js 16+ and use Typescript 4.x. Prettier docs note that "all" requires Node 8+ and Typescript 2.7+.

[ephemeralrogue]

@pythonmcpi, eslint:recommended looks like it should be fine. The only changes I'd recommend is setting indents to tab instead of four spaces--where a tab is set to the length of four spaces--to make jumping around a little easier when writing; and using single quotes where quotes are necessary. Everything else looks good.

Same with Prettier, though I'm beginning to think that Prettier isn't really necessary if ESLint can fix linting errors. We can probably remove that package too and just specify the semicolon and trailing comma rules in the ESLint config, and create a linting script.

Print width may be better set to a narrower amount, ie between 80 & 100, as this will accommodate developers working on different kinds and sizes of machines. This will also force nesting, which in many cases can improve the readability of code. One line if statements are simple enough, but terniary operators can get cumbersome when allowed to run rampant, as an example.

We def don't need separate linting rules and configurations between prod and dev environments. We can consolidate a lot of this by removing .dev.* and nested configuration files. Let's put all configuration at the root, and resolve any issues that arise as a result of doing so.

[pythonmcpi]

I agree with all of your points. However, I think prettier is worth keeping, as it's primarily an autoformatter rather than a linter. I use the Prettier plugin for vscode and it makes formatting easier. Eslint is meant as a linter and cares more about what your code does rather than whitespace/punctuation.

[ephemeralrogue]

I think prettier is worth keeping, as it's primarily an autoformatter rather than a linter. I use the Prettier plugin for vscode and it makes formatting easier. Eslint is meant as a linter and cares more about what your code does rather than whitespace/punctuation.

Got it. Works for me. Let’s do it 👍

[ephemeralrogue]

@pythonmcpi, do you want to submit a PR with these changes?

[pythonmcpi]

I'll be busy for the next few days, so if you want to perform these changes go ahead. If not, I can submit a PR.

[ephemeralrogue]

@pythonmcpi, I started working on an update for this on this branch. As I started cleaning out configuration files, I started running into all kinds of issues removing dependencies. As a result, I scrapped all of commitzen, commitlint, semantic-release, everything eslint, everything jest, and typescript and started from scratch. I bumped TypeScript to 5.x and Jest to 29.x and worked through most of the files with the configuration I thought made the most sense.

I'll be unable to work on this over the next couple of days. If you want to review the branch and provide any notes, I'm welcome to them. You're also welcome to pull the branch and contribute to it directly if you like. If you decide that what I'm proposing is absolute madness, you're also welcome to start work on your own branch and we can discuss the changes as you go. I'm open to collaborating, or you can run with it if you're more comfortable with that.

The ultimate goal with this work is to ease contributor onboarding to the codebase, and make it so that it passes linting and the damn thing still builds, as @tonyxforce put in a decent amount of work to at least bring it to that point.