buildinamsterdam / lint

BiA's base lint config(s).
MIT License
0 stars 0 forks source link

Loosen rules to prevent breaking builds #13

Closed moritzsalla closed 1 year ago

moritzsalla commented 1 year ago

Want to kick off a discussion about how strict we want to be in enforcing rules.

Setting rules to "error" will cause builds to break, slowing down work in progress and cluttering PRs.

Would be nice to strike a better balance between enforcing practices and productivity. I'd propose we go for "warn" on rules that are "cosmetic" and don't present a bug.

moritzsalla commented 1 year ago

Interesting idea! Guess for me the first thing that comes to mind is that currently all rules that we enforce with the error are auto linted. So in theory none should actually block a build? Simply running an npm run fix (alongside lint on save) should solve all issues relating to the linter (par the new enum rule that is)?

We do get braking builds in feature PRs when the linter fails on bezier. No override build command set on vercel. Just the regular next build in package.json.

So my guess is next runs the linter during it's build process, unless I'm missing something big 😃

PauloMFJ commented 1 year ago

Interesting idea! Guess for me the first thing that comes to mind is that currently all rules that we enforce with the error are auto linted. So in theory none should actually block a build? Simply running an npm run fix (alongside lint on save) should solve all issues relating to the linter (par the new enum rule that is)?

We get braking builds in feature PRs on bezier. No override build command set, just the regular next build. So my guess is next runs the linter during it's build process, unless I'm missing something big 😃

Yeahh, next builds run next lint under the hood on all builds. So only ESLint rules are picked up for select files that Next decided. Even though we often include md etc files in our custom linting rules they're ignored by the build.

In any case and in theory: If you have lint on save setup and working, you should mostly never get linting breaking your builds. With that said the main exception that I often run across that breaks builds is when I accidentally push up console.log to the build. Do you have other examples of when you run across linting issues frequently?

MaartenBruggink commented 1 year ago

Yeah interesting, I'd say ideally you'd have different behaviour;

Not sure if this is feasible though. If not I'd say we'd probably need to add husky with a nice pre-commit lint:fix to our next template. And then I just need to not save that often while working 😄

PauloMFJ commented 1 year ago

Yeah interesting, I'd say ideally you'd have different behaviour;

  • In the editor you'd want warnings/errors but on save I don't want these things to be fixed automatically. While working on things I'd prefer to not have my just added import removed that I still needed to implement
  • But pre-commit I do want it to clean up my mess

Not sure if this is feasible though. If not I'd say we'd probably need to add husky with a nice pre-commit lint:fix to our next template. And then I just need to not save that often while working 😄

Yeahh would be a nice option if you could auto fix things on commit. Sadly from past experience I find pre-commit hooks like Husky just slow things down. If it was instant - great, but having to wait for a script to run just becomes tedious on every commit.

moritzsalla commented 1 year ago

Now that we are using a more complex eslint setup + typescript, my mac likes to slow down so much that it starts queuing eslint tasks which chokes the live server. At that point i usually have to restart my laptop.

I'd rather wait a second and reduce some load during saves. But that might just be a problem of the folks still on the Intel macs. 🤓

PauloMFJ commented 1 year ago

Now that we are using a more complex eslint setup + typescript, my mac likes to slow down so much that it starts queuing eslint tasks which chokes the live server. At that point i usually have to restart my laptop.

I'd rather wait a second and reduce some load during saves.

But that might just be a problem of an older breed still on the Intel macs. 🤓

Very frustrating. I've experienced that before and still do from time to time. I'm actually not sure if this is even really down to a more complex setup as much as it is just ESLint being slow from time to time. Small tip: I find that restarting the ESLint server usually solves the issue for me: CMD + P then ESLint: Restart ESLint Server.

moritzsalla commented 1 year ago

Nice one! I'll try next time.

MaartenBruggink commented 1 year ago

Yeah I agree with Moritz on this one. If we have a strict setup, disable the eslint on save action and setup a pre-commit hook we'd have the situation that I described earlier;

Sounds like a good implementation to me 🦦

PauloMFJ commented 1 year ago

Yeah I agree with Moritz on this one. If we have a strict setup, disable the eslint on save action and setup a pre-commit hook we'd have the situation that I described earlier;

  • VSCode gives comments about me being sloppy
  • Won't be annoying on save unless I want it to run: cmnd/shift + p -> Eslint: all auto-fixable Problems
  • Pre-commit it will auto align everything on my staged changes

Sounds like a good implementation to me 🦦

Think this is very much a personal preference thing tbh. I quite like the experience of getting linting on save given it's instant 🤷 My coding style is very much; push little but often and find Husky slows down the process personally. Whenever I've used Husky it's just become more of an hindrance.

For the type of experience your looking for, I reckon there is probably ways you could achieve that?

MaartenBruggink commented 1 year ago

Let's move this conversation to the boilerplate since it's more relevant on the config there 👍

moritzsalla commented 1 year ago

It's been great lads, let's carry this on "over there".