lume / glas

WebGL in WebAssembly with AssemblyScript
https://lume.io
MIT License
887 stars 45 forks source link

ESLint #109

Closed StEvUgnIn closed 1 year ago

StEvUgnIn commented 3 years ago

Prettier is good at formatting. But it leaves open for contributions and tests to be deployed without respecting project rules.

I propose to add and configure ESLint to include our project rules and add no-var-keyword rules and also ES5 rules about comma.

trusktr commented 3 years ago

Even without ESLint yet, we can add prettier --check to the test scripts at

https://github.com/lume/glas/blob/04a178b23788a79e4b4f88c55d9944dee1fff481/package.json#L12-L14

and it will be called by CI

https://github.com/lume/glas/blob/04a178b23788a79e4b4f88c55d9944dee1fff481/.github/workflows/nodejs.yml#L26

so PRs will fail if code isn't formatter.

In some other projects, I use ESLint + Prettier.

Husky + lint-staged is great too.

It would be sweet to set up ESLint + Prettier (and any additional rules) along with Husky + lint-staged.

trusktr commented 3 years ago

All the other LUME packages use the same scripts, for example, which come from @lume/cli.

Only lume/glas currently doesn't because it is so different, relying on asc and as-pect.

But for stuff like ESLint and Prettier, it would be common in all the projects. It would be nice to add this to lume/cli and have all projects linted during their tests using the same commands from the cli.

p3nGu1nZz commented 3 years ago

was working on implementing that, but need to support the cli to work on my windows machines. wip..

trusktr commented 3 years ago

will try to dig up the window machine tomorrow.

StEvUgnIn commented 3 years ago

Personally, I am more in inclined toward a ready-to-use solution instead of describing any linter configuration (package.json is universal for instance)... Deno comes with a formatter. If Visual Studio code is mandatory for contributing to the project, then we should write formatting rules inside VS code project configuration files.

StEvUgnIn commented 3 years ago

We could edit the gitignore as such: https://github.com/github/gitignore/blob/master/Global/VisualStudioCode.gitignore

p3nGu1nZz commented 3 years ago

You can configure checkstyle rules for github to enforce the lint and style rules., TODO; Commiting editor configurations is not practical. I generally only use Visual Studio Code to smoke test and code review. I use VIM, notepad++, and Sublime equally. I think its best to rely on our lint and prettier configurations. You can also adjust your VS Codes to match the project configuration so you can format the document and get the same results.

p3nGu1nZz commented 3 years ago

I think we should revisit

All the other LUME packages use the same scripts, for example, which come from @lume/cli.

Only lume/glas currently doesn't because it is so different, relying on asc and as-pect.

But for stuff like ESLint and Prettier, it would be common in all the projects. It would be nice to add this to lume/cli and have all projects linted during their tests using the same commands from the cli.

also maybe create a some checkstyle unit tests / github action that does this. If the test fails then the PR wont be able to be merged.

trusktr commented 3 years ago

Yeah, glas is the oddball of the LUME projects being so different. The others are all regular TypeScript, whereas glas is the only AssemblyScript project so I hadn't copied the same repo format. (Actually I'd like to make a TypeScript/AssemblyScript hybrid setup, so each project that qualifies can be compiled to either AS or JS without the same configs and processes from the cli).

Yeah like Kara mentioned, people use other editors besides VS Code. I sometimes go back to Atom, or NeoVim (especially on my server over SSH). The configs and commands in the repo will be the source of truth, and every well-known editor has Prettier/ESLint/EditorConfig/etc support.

Been meaning to setup up commit checks with lint-staged and husky packages. Works really well: we can auto-format code on each commit, even if we're using only a plain text editor.

Got my Windows laptop ready now, so I'll be getting to testing it all in Windows soon. It should work in any OS (if we set up the the package.json scripts correctly).