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

build: add Prettier support and configurations #8220

Closed
wants to merge 1 commit into from

Conversation

trumbitta
Copy link
Contributor

@trumbitta trumbitta commented Feb 15, 2022

Description

Opening this new one since #7757 was too much.

Why

  • I'm about to start working on fixing the linting problems the dashboard app logs [dashboard] Lint warnings #3841, but ESLint and Prettier have some common gray area and it's better to start with consistently formatted code to have an easier time in handling this common area
  • Basically why Prettier exists: having an automated coding style which is readable, consistent, and not particularly prone to be argued upon: again, @loujaybee this improves DevX
  • I just took a look at the codebase (not just at the dashboard) and I found inconsistent formatting (file A with double quotes, file B with single quotes, file C with both, same for indentation etc).

This makes contributing harder than it should be. It's either contributors having to format their code without automated formatters, or having PRs full of irrelevant style changes with the actual feature or fix hidden somewhere.

But don't take my word for it: https://prettier.io/docs/en/why-prettier.html

What

  • Adds Prettier support and configurations
  • Activates format on save for the VSCode workspace
  • Prettier is not invasive, it doesn't change how code works (that's ESLint), just how it looks, it doesn't break anything (that's potentially again ESLint)

Disclaimer

This is the minimum viable contribution to get things going in the right direction and over time get to a point where all the code is formatted the same way and code formatting is automated.

Its biggest con is that after you merge this, for months, every single PR of code written with VSCode or another IDE with active Prettier support will be full of irrelevant - but much needed for the reasons stated above - code formatting changes.

Formatting all at once (again, this is not linting, it doesn't change how the code works and it's supposed to be safe), instead, gives you a solid starting point and doesn't impact future PRs.

Related Issue(s)

Fixes #

How to test

Edit a TS (for example) file in VSCode and save it. You should see the formatting change, unless your own user settings specifically turn off format on save and Prettier support.

Release Notes

Make it easier to contribute to Gitpod by automating code formatting via Prettier

@geropl
Copy link
Member

geropl commented Feb 28, 2022

IMO the best path to migrate an existing code base to:

  • decide on linting rules: ✅
  • auto-format enabled (like this PR) ✔️
  • make it opt-in on a per-file basis: this way the migration is done over time, and on the other hand guaranteed to stay stable. I really really want to avoid this to interfere with other work. Prettier seems to support this. ✅

What do others think?

@trumbitta
Copy link
Contributor Author

trumbitta commented Mar 4, 2022

IMO the best path to migrate an existing code base to:

  • decide on linting rules: ✅
  • auto-format enabled (like this PR) ✔️
  • make it opt-in on a per-file basis: this way the migration is done over time, and on the other hand guaranteed to stay stable. I really really want to avoid this to interfere with other work. Prettier seems to support this. ✅

What do others think?

While we wait for the other Gitpodders to chime in, I'd like to add that I already went this route in the past with a project of comparable size, and it worked in the end but with the cons I wrote about in the description of this PR.

At the end of the day, the repository is yours and I'm just happy to help so the decision is all yours and I'm here for y'all 😊

@easyCZ
Copy link
Member

easyCZ commented Mar 10, 2022

I've landed #8655 which adds a minimal prettier config and setups pre-commit to run on git commit as a hook.

@trumbitta
Copy link
Contributor Author

oh, ok...

@trumbitta trumbitta closed this Mar 10, 2022
@JanKoehnlein
Copy link
Contributor

Thanks for your contribution @trumbitta and for pointing us to this!

@trumbitta
Copy link
Contributor Author

I guess this means I can finally start working on #3841 if that's still available? @JanKoehnlein @geropl @easyCZ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants