-
-
Notifications
You must be signed in to change notification settings - Fork 94
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
Improve Prettier, formatting enforcement #102
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@sammoore is attempting to deploy a commit to the ronin-tech Team on Vercel. A member of the Team first needs to authorize it. |
@sammoore can you elaborate on the purpose of upgrading from v2 to v3 of Prettier? |
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.
Will want to get those build steps to succeed before merging
@danstepanov Thought it made sense to get everything up to date and on the same version, since If we're running Prettier on commit or from our editors, it's possible in these scenarios only one version or the other will be used. So by upgrading, we avoid the possibility that an unexpected version (for either project) is used by upgrading so they are all using v3. |
b41542e
to
266622f
Compare
266622f
to
e0e0c08
Compare
Rebased and updated with my latest changes and with the builds resolved (I think by upgrading bun and then updating the lockb file?). A few pending things:
|
@sammoore yea can you break the PR up if it addresses different things? |
"Half" of this PR (Husky fixes & the automated After that's merged, will rebase and update this PR with just the Prettier/tabSize/etc changes. |
e0e0c08
to
fa9f0ef
Compare
For the curious: updating the PR with rebased commits (against the other portion of this original PR that have been merged in #125). Still need to:
|
I've moved the This PR description is updated, but the PR will need to be rebased again after #126 is merged. |
f5976b8
to
1ef90f1
Compare
Rebased against #126 and #125. Still need to verify changes locally (empty checkbox here) and make sure nothing else is needed. |
1ef90f1
to
cab9f5d
Compare
cab9f5d
to
e43286e
Compare
pretty-quick (previously used) has not been updated in 18 months, and is now broken on newer versions of prettier. We use lint-staged instead to run prettier on the codebase pre-commit. See: https://github.com/azz/pretty-quick/issues/168w
e43286e
to
9f5b0f3
Compare
Removed the change from tabs to 2 spaces. Will include in the PR that actually applies the switch. Also, updated the PR description to mention Rebased and ready for review. |
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
This PR updates prettier, runs it on the pre-commit hook, and updates the VSCode/editorconfig files to ensure settings are used regardless of the workspace used in the editor.
lint-staged
to run Prettier on the relevant committed files upongit commit
.pretty-quick
, which hasn't been updated in about 18 months and does not work with Prettier v3, which the majority of this project does/did utilize (see next bullet).cli/.vscode/settings.json
) to.vscode/settings.json
{cli,docs,www}/.vscode/settings.json
to the top level settings.json, to ensure the settings are used regardless of the folder that's opened as the workspace..vscode/extensions.json
for recommended/unwanted extensions, and use similar symlinks as above for sub-projects..editorconfig
for generic editor support for line endings and tab style/sizing.