devedmonton / DES-Website

The Dev Edmonton Society website! We empower Edmonton Developers!
https://devedmonton.com
MIT License
26 stars 65 forks source link

Update prettier and eslint setup #217

Closed DerrykBoyd closed 1 year ago

DerrykBoyd commented 1 year ago

What issue is this referencing?

This brings the prettier and eslint setups up to date with how the tools are recommended to be used.

Remove prettier rules form eslint and run prettier separately as per https://prettier.io/docs/en/integrating-with-linters.html

We should add "npm run format-check" to this issue template

Include js files in linting (was only vue files before)

Fix the formatting on files that were not covered in prev settings.

Have you run the linting command in the README.md?

Have you taken a look at our contributing guidelines?

My node version matches the one suggested when running nvm use?

DerrykBoyd commented 1 year ago

Why does it change whether we have 2-4 spaces depending on the file type? Is that normal? I thought they were always project wide decisions?

I think that's a specific instance with code blocks inside markdown getting some different formatting. I'm not totally sure though, I tend not to pay much attention to the formatting, just set it and forget it 😄

This does use the default prettier settings project wide, but before we were formatting with eslint which was limited to only .vue files so some files like markdown and json were getting missed. Those are updated and included now.

DerrykBoyd commented 1 year ago

Why does it change whether we have 2-4 spaces depending on the file type? Is that normal? I thought they were always project wide decisions?

On another look, we have a .editorconfig file that overrides some of the prettier settings. That's where this difference is coming from. So just to confirm

I would be up for just removing the editorconfig file and defaulting to the default prettier. That would require a format commit that would change pretty much all files.

MandyMeindersma commented 1 year ago

I feel like that's too much effort ahhahah

I am good to merge as is :P

DerrykBoyd commented 1 year ago

I feel like that's too much effort ahhahah

I am good to merge as is :P

Ha yeah, I'm easy. Not much effort but let's just leave it as is. Will keep the git history cleaner.

MandyMeindersma commented 1 year ago

so we should add npm run format-check to the readme and the pr template? they are both in the repo

olimpiuus commented 1 year ago

@DerrykBoyd, that's a fantastic commit! Could we also consider adding something like the following?

"lint-fix-and-format": "eslint . --ext .js,.vue --fix && prettier --write ."

This script could be quite useful for automatically fixing linting issues and formatting the code. However, I'm unsure about its appropriateness. What do you think?

DerrykBoyd commented 1 year ago

Yeah sure, I don't see any issue with that. Not something I'd use since my editor is now formatting / fixing on save. But if it's useful for others there's no harm.

Would suggest

  "lint-fix-and-format": "npm run lint-fix && npm run format",

So we don't have duplicated commands to maintain