Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format JS with Prettier #16

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Format JS with Prettier #16

merged 1 commit into from
Jul 18, 2024

Conversation

irskep
Copy link
Contributor

@irskep irskep commented Jul 14, 2024

Having a .prettierrc means anybody who uses format-on-save will automatically follow project conventions. I used 2-space tabs because that's what you put in .editorconfig, and otherwise just left all the default settings alone.

A step further would be to add a CI job which fails if the submitter doesn’t use the formatter, but I'll let you decide if you want to go that far. I'd be happy to write the config file to make that happen.

I included a vscode config to make it more likely that future contributors do the right thing. I didn't realize until #17 that it also formats CSS, so hopefully that's OK. I can scope it down to just JS if necessary.

@irskep irskep force-pushed the js-formatting branch 2 times, most recently from 63765e6 to 02b3bf1 Compare July 15, 2024 01:55
@irskep
Copy link
Contributor Author

irskep commented Jul 15, 2024

Tacked on a commit (with a good message) doing the camelCase conversion, and converting 'var' to 'let', since 'var' is essentially deprecated now. Figured it would be better to do all the trivial formatting changes in one go.

@cu
Copy link
Owner

cu commented Jul 15, 2024

Thanks for doing this.

The more that I look at it, the more I'm unsure about switching all the JS to 2-space indentation. I know that's the de facto standard pretty much everywhere now. And maybe it looks fine for someone who has been steeped in Javascript for a good long while, but I tried to give it a chance by reading the modified sections in github and also in vscode, but it's just not working for me. I find it harder to read and sadly I can use all the help I can get!

I'm guessing the last section in .editorconfig including *.js was either an oversight or overenthusiastic aspiration. That said, I'm fine with adding .prettierrc and the vscode config, and letting fix up the other formatting issues.

So, I think I can accept this, with the following changes:

  1. Adjust .editorconfig js indent to 4. (Just remove it from the glob I think?)
  2. .prettierrc tabWidth to 4
  3. Drop the indentation-only changes but let Prettier have its way with the rest of the formatting.

@cu
Copy link
Owner

cu commented Jul 15, 2024

I am okay with the second commit but I think it should be its own PR.

@irskep
Copy link
Contributor Author

irskep commented Jul 15, 2024

Oh I'm completely fine with 4-space tabs, I was just going by what was there already. I don't think there's a firm standard one way or the other. Will make the changes.

Having a .prettierrc means anybody who uses format-on-save will automatically follow project conventions.
@irskep
Copy link
Contributor Author

irskep commented Jul 15, 2024

Updated!

@cu cu merged commit ed43f5e into cu:master Jul 18, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants