search

inventaire / inventaire-client

webapp coupled to the inventaire server :books:
https://inventaire.io
48 stars 16 forks source link

Add stylelint #378

Closed jum-s closed 1 year ago

jum-s commented 1 year ago

After quite some time trying to configure prettier, i can say that their non-modifiable opinionated configuration is bad (im not the only one complaining ).

So instead, the last commits setup and fix with stylelint. Rules are soft, as CSS readability is less important than JS).

maxlath commented 1 year ago

@jum-s it seems that the commit that added scripts/stylelint.cjs disappeared

jum-s commented 1 year ago

I dont think there was any integrated scripts before. IDE extension uses default config file. I added something in 338e4f39f but not sure its worth the time spent.

maxlath commented 1 year ago

I think it's useful to have it in pre-commit hooks, similarly to eslint, or generally to have it as an available script to be easily run on the whole codebase, in case we change some rules in the future.

jum-s commented 1 year ago

Hum, I was not expecting that in depth look at stylelint. Actually, im having second thought on this PR. The customization your mentioning in the comments above is quite some work for relatively small DX advantages. And its adding new conventions where CSS looked readable to me beforehand.

I didnt mention enough my intention when open the PR: i thought i would introduce CSS linting smoothly/lightly, without enforcing much. That's why there was initially no scripts and there is still no .scss coverage.

I pushed a few updates taking most of your comments above into account. But i would propose to leave it configured by default with stylelint-config-standard-scss setup. And maybe a more strict setup can wait a bit (or another PR)?

I wont do much more on this PR, and Im also fine not merging it, which would leave stylelint as custom local settings, and it would be fine for me.

maxlath commented 1 year ago

@jum-s I do see quite some DX improvement from it. I can take over this PR if you lost interest, or merge it like this and propose further customization in a follow-up PR.

jum-s commented 1 year ago

Should be ready for review and merge. i might take a little while to review stylelint PR (as i had my share of it for now), but you are welcome to open another PR for some fine tuning.