-
Notifications
You must be signed in to change notification settings - Fork 6
Feature 2230 Prettier #2231
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
Feature 2230 Prettier #2231
Conversation
| "universal-cookie": "^4.0.4" | ||
| }, | ||
| "scripts": { | ||
| "start": "vite", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I alphabetized the scripts.
| "serve": "vite preview", | ||
| "test": "vitest", | ||
| "coverage": "vitest run --coverage --watch=false", | ||
| "format": "prettier --write 'src/**/*.{css,html,js,jsx}'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only new script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include tsx for futureproofing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking the same thing, and to include .ts, .cjs and possibly .mjs as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I merged before I saw these comments. I will add those extensions in another PR.
| "jsdom": "^24.0.0", | ||
| "msw": "^2.2.13", | ||
| "prettier": "2.8.1", | ||
| "prettier": "3.2.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the latest version of Prettier.
| } | ||
|
|
||
| @media print { | ||
| :root { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of formatting changes follow!
I ran the app locally and didn't see any issues.
I also tried to verify that all the UI tests still pass. 7 failed. I'm investigating those now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to update some snapshots related to whitespace. Now all the UI tests pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
jackkeller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I added a comment but it's not a blocking comment.
|
For anyone who see's the 301 file changes - aside from snapshot tests these are mostly formatting like: single ticks versus double, no trailing commas, and some whitespace changes so not a lot would affect the app as a whole, more so the developer experience. |
|
@mvolkmann you had two conflicts that were from my story, I fixed them up because I knew exactly what was needed. Cheers! |
jackkeller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
vhscom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
This PR contains the results of running Prettier on all the UI files.