Skip to content

Use prettier for TypeScript files in .werft #8245

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

Closed
wants to merge 2 commits into from

Conversation

mads-hartmann
Copy link
Contributor

@mads-hartmann mads-hartmann commented Feb 16, 2022

Description

This PR introduces automatic formatting of all TypeScript files in the .werft folder.

  1. It configures prettier to perform automatic formatting on all TypeScript files in the .werft folder as part of our pre-commit hooks.
  2. It adds esbenp.prettier-vscode to the list of extensions
  3. It configures VSCode to fun "format on save" on TypeScript files and sets prettier as the formatter

This PR also includes performing the automatic formatting, hence the rather large diff. For the actual code review I suggest you focus on 43ca5ee.

Related Issue(s)

Fixes #8066

How to test

Verify that it performs formatting when you run

pre-commit run --all-files --show-diff-on-failure

Edit a ts file in server and verify it doesn't use prettier to format the file when saving (that is, the prettier extension that is now installed as part of the workspace correctly ignores the entire components folder)

Release Notes

NONE

Documentation

N/A

@github-actions
Copy link
Contributor

⚠️ Hey reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yml was changed and it might be harmful.

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #8245 (2094d05) into main (ce70183) will decrease coverage by 20.03%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #8245       +/-   ##
===========================================
- Coverage   31.21%   11.17%   -20.04%     
===========================================
  Files          39       18       -21     
  Lines        5908      993     -4915     
===========================================
- Hits         1844      111     -1733     
+ Misses       3924      880     -3044     
+ Partials      140        2      -138     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
components-supervisor-app ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/supervisor/pkg/terminal/service.go
components/supervisor/pkg/ports/exposed-ports.go
...mponents/supervisor/pkg/supervisor/notification.go
components/supervisor/pkg/ports/ports-config.go
components/supervisor/pkg/terminal/ring-buffer.go
components/local-app/pkg/auth/pkce.go
components/supervisor/pkg/supervisor/services.go
components/supervisor/pkg/supervisor/supervisor.go
components/supervisor/pkg/supervisor/git.go
components/local-app/pkg/auth/auth.go
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce70183...2094d05. Read the comment docs.

@mads-hartmann mads-hartmann marked this pull request as ready for review February 17, 2022 09:14
@mads-hartmann mads-hartmann requested a review from a team February 17, 2022 09:14
@mads-hartmann
Copy link
Contributor Author

I have added the hold label. I'm checking with the other teams that write TS to ensure this doesn't break anything for them.

@ArthurSens
Copy link
Contributor

ArthurSens commented Feb 17, 2022

Nice job! I'd approve right away if the build didn't fail... looks like it is some unrelated flakiness though, I'll rerun to see if it passes

/werft run

👍 started the job as gitpod-build-mads-prettier-werft.5

@ArthurSens
Copy link
Contributor

ArthurSens commented Feb 18, 2022

What is the status of this one?

Are we still waiting for IDE/WebApp review? If that's the case I guess we could add them as reviewers? 🙂

@mads-hartmann
Copy link
Contributor Author

mads-hartmann commented Feb 21, 2022

@ArthurSens I will try to use the default formatter that VSCode ships with instead of prettier. I can't figure out how to downgrade a PR to DRAFT so instead I'll just close this and reopen a new one once I have experimented ☺️

@ArthurSens
Copy link
Contributor

Right top corner :)

image

@mads-hartmann
Copy link
Contributor Author

@ArthurSens Thanks! Reopened and converted it to draft :D

@stale
Copy link

stale bot commented Mar 6, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Mar 6, 2022
@mads-hartmann mads-hartmann added the meta: never-stale This issue can never become stale label Mar 7, 2022
@stale stale bot removed the meta: stale This issue/PR is stale and will be closed soon label Mar 7, 2022
@mads-hartmann
Copy link
Contributor Author

Closing this, the webapp team is working on adding automatic formatting so I'd rather wait and piggy-back on their implementation. See internal Slack thread for reference

@mads-hartmann mads-hartmann deleted the mads/prettier-werft branch March 9, 2022 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run prettier formatting and test for formatting in builds
3 participants