Skip to content

Repo: Git hooks should run Prettier only on changed files in commits #5203

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.

Sign up for GitHub

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

Jump to bottom
Closed
JoshuaKGoldberg opened this issue Jun 19, 2022 · 4 comments · Fixed by #5235
Closed

Repo: Git hooks should run Prettier only on changed files in commits #5203

JoshuaKGoldberg opened this issue Jun 19, 2022 · 4 comments · Fixed by #5235
Labels
accepting prs Go ahead, send a pull request that resolves this issue repo maintenance things to do with maintenance of the repo, and not with code/docs

Comments

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Jun 19, 2022 •
edited
Loading

Suggestion

Right now the pre-push hook in husky runs Prettier on everything, which takes many seconds:

$ git push
yarn run v1.22.19
$ yarn check-format
$ prettier --list-different "./**/*.{md,mdx,ts,mts,cts,js,cjs,mjs,tsx,jsx}"
Done in 19.43s.
Everything up-to-date

The repository already has a pre-commit hook for formatting. It should be impossible for users to add poorly formatting code unless they manually --no-verify. Is there a reason to keep pre-push in addition to pre-commit?

Related: #445, #556, #5060
@JoshuaKGoldberg JoshuaKGoldberg added triage Waiting for team members to take a look repo maintenance things to do with maintenance of the repo, and not with code/docs labels Jun 19, 2022
@Josh-Cena
Copy link
Member

+1... The only reason I came up with—if there ever is one—is because Prettier is not idempotent in all cases (?)

@bradzacher
Copy link
Member

If prettier is not idempotent in a casr then it's a bug to report to prettier.

It supposed to be deterministic and stable.

@Josh-Cena
Copy link
Member

Josh-Cena commented Jun 19, 2022 •
edited
Loading

Prettier already has a long list of non-idempotency issues. Not always easy to fix, though.

Also, after 2.7, there's the --cache option, which theoretically should eliminate the performance issue.

@JoshuaKGoldberg
Copy link
Member Author

Cool I'll just mark this as accepting PRs then 😄. I've had the proposed system in most of my other repos for a while and haven't had any issues.

@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Jun 23, 2022
Merged
3 tasks
@bradzacher bradzacher added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Jun 23, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue repo maintenance things to do with maintenance of the repo, and not with code/docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: removed pre-push hook JoshuaKGoldberg/typescript-eslint
3 participants
@JoshuaKGoldberg @bradzacher @Josh-Cena