Prettier + ESlint + Github Actions

[quolpr]

Before going to typescript support, I want to setup prettier and eslint first 🙂 I also added CI support(Github Actions) on any pull requests or pushes

[quolpr]

As for the prettier, I used the default config. But I am good if we want to adjust some of the settings

[rshigg]

I think the prettier config should affect the existing code as little as possible. In this case it seems like just setting singleQuote to true should do the trick.

What do you think about also setting up a pre-commit hook for eslint+prettier using husky? https://github.com/typicode/husky

[quolpr]

Yep, good ideas 🙂 Let me do

[quolpr]

Done. I also wanted to add just to the GitHub actions, but tests are failing. @jlongster could you fix them? And then I will create another PR with the GitHub actions jest support

[jlongster]

Thank you so much! This is great.

I'm really not a fan of pre-commit hooks. I make small commits all the time, and the majority of them are in-progress commits that fail eslint sometimes. IMHO, git commit should be as fast and simple as possible.

Let's lean on CI to enforce that eslint passes and prettier has formatted the code, and provide the same checks with a single yarn can that you can run before pushing. And provide a format command that makes prettier format all files in the codebase if something is wrongly formatted. (Contributors are expected to have prettier setup in whatever code editor they are using so that shouldn't need to be used much)

[jlongster]

I'm on PTO today and tomorrow so won't be able to get the tests passing until Friday or Saturday. I'm happy to also start the TS integration if I have time.

[quolpr]

I played around, it is pretty easy to add TS support 🙂 Once this PR merged I will make another PR with the TS setup, and we can continue work on moving the project to TS

[quolpr]

@jlongster also, fair points. I will make changes

[rshigg]

I'm really not a fan of pre-commit hooks. I make small commits all the time, and the majority of them are in-progress commits that fail eslint sometimes. IMHO, git commit should be as fast and simple as possible.

I'm also not a fan of pre-commit linting but it seems to be the standard these days. Maybe the best way to go is to just run prettier on push? I highly recommend some sort of git hook for formatting in a project like this (leaving linting to the CI is a good idea).

[quolpr]

TBH, as for me, I tend to avoid pre-commit hooks. We have a pre-commit hook on the project where I work with the team, but I disabled it personally. CI will show you that linting fails, plus I love fast commit too.

But on the other hand, we are using lint-staged, that will lint only changed filed, that increase the recommit time dramatically 🤔. Not sure about push cause it will lint all the files(not just that are changed), which may slow down the push time. For me, it's easier if I push and go to the next task(and then got notified that CI failed and fixed when I have time), then await the push.

TBH, it's a matter of personal preference 🤔

[bartosz-k]

I'm also not a fan of pre-commit linting but it seems to be the standard these days

I haven't heard of this standard, it must be per project. Btw, there is one caveat with pre commit hooks - what if there is a bug in code being run during commit hook or... this code gets infected by malicious code. You can loose your uncommited changes which sometimes can be many hours of work.

[quolpr]

and provide the same checks with a single yarn

As I understand, the single yarn is always about packages installation.

So I decided to make it as it has done in the prettier package — https://github.com/prettier/prettier/blob/7bda3a3a58392fa9469c9812fa2059bbc0840ea9/package.json#L153-L164

[quolpr]

@jlongster could you check? The CI is ok btw https://github.com/quolpr/absurd-sql/runs/3988469188?check_suite_focus=true

[joekukish]

@quolpr why separate eslint from prettier? Ideally prettier is just a plugin from eslint so they are executed together and there's no need to separate the commands. You can have lint and lint:fix and that's it.