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

update prettier and clang-format cli options in format.py #2576

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Aug 21, 2024

  • Replaced --check with --list-different to reduce noise (hides the files that does not need formatting)
  • Added --log-level=warn on formatting where now it prints only the files that actually changed.

@anonrig anonrig requested review from npaun and danlapid August 21, 2024 15:49
@anonrig anonrig requested review from a team as code owners August 21, 2024 15:49
@danlapid
Copy link
Contributor

Is this for taste or performance?
If it's for the latter, what kind of difference are you seeing in formatting time?

@npaun
Copy link
Member

npaun commented Aug 21, 2024

If it's just to cut down on noise I think --check + --log-level=warn will look nicer than --list-different?

@anonrig
Copy link
Member Author

anonrig commented Aug 21, 2024

Is this for taste or performance? If it's for the latter, what kind of difference are you seeing in formatting time?

Mostly for taste. I don't have githooks enabled, and I run python3 tools/cross/format.py manually, which just forces me to clear my terminal everytime I run it.

My local benchmarks using hyperfine -i "node_modules/.bin/prettier . --check" "node_modules/.bin/prettier . --list-different" shows that --list-different is 3-5% faster but I don't think that's really my reasoning for this pull-request.

@hoodmane
Copy link
Contributor

I agree with @anonrig that we should get rid of clang-format --verbose. I don't want the formatting command to generate multiple screens worth of output.

@jasnell
Copy link
Member

jasnell commented Aug 21, 2024

+1 to going with the less verbose output

@anonrig anonrig changed the title update prettier cli options in format.py update prettier and clang-format cli options in format.py Aug 21, 2024
@anonrig
Copy link
Member Author

anonrig commented Aug 21, 2024

Would you mind reviewing this pull-request?

tools/cross/format.py Outdated Show resolved Hide resolved
tools/cross/format.py Outdated Show resolved Hide resolved
@anonrig anonrig merged commit 52f709f into main Aug 21, 2024
10 checks passed
@anonrig anonrig deleted the yagiz/prettier-changes branch August 21, 2024 18:28
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.

5 participants