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

Run prettier as part of ESLint #1676

Closed
wants to merge 3 commits into from
Closed

Conversation

koddsson
Copy link
Member

This way we get a failure in CI if the code isn't formatted according to prettier.

@koddsson koddsson requested a review from a team as a code owner February 23, 2025 12:25
@koddsson koddsson enabled auto-merge (squash) February 23, 2025 12:25
@43081j
Copy link
Contributor

43081j commented Feb 23, 2025

i actually wouldn't do this

i'd just prettier --check in an npm script

one day really, prettier's eslint plugin/config should go away. since it should just be an automated thing run at the end

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
"lint:js": "eslint lib/",
"lint:format": "prettier --check lib",
"lint:js": "eslint lib/ && prettier --check lib/",
"lint:format": "eslint lib/ --fix && prettier --write lib/",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means the lint script will run both of them twice. I guess you meant to also update the lint script to only call lint:js?

Maybe we can make it clearer by calling the other lint:fix instead

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh maybe I've misunderstood how this works. I've been getting prettier changes in my PRs that didn't fail CI so I assumed it wasn't working but I can see now that it should be failing the build.

Maybe I was looking at the wrong thing then. I'm just gonna close this and add a git hook I think.

@koddsson koddsson disabled auto-merge February 24, 2025 09:12
@koddsson koddsson closed this Feb 24, 2025
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.

None yet

2 participants