Skip to content

Conversation

jaudiger
Copy link
Contributor

Based on top of #337

This PR is very similar to #255, it adapts the format command this time to be more efficient when working with multiple projects.

@jaudiger jaudiger self-assigned this Oct 10, 2025
kylewlacy
kylewlacy previously approved these changes Oct 10, 2025
Copy link
Member

@kylewlacy kylewlacy left a comment

Choose a reason for hiding this comment

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

LGTM

I tested locally with this command in brioche-packages:

echo ./packages/* | xargs printf -- '-p %s\n' | time xargs brioche fmt

...and it didn't seem to speed up or slow down things noticeably (~7 seconds before and after this change). But the end result has a better output in my opinion, since it now shows all the files at once instead of separating them by project, which I personally prefer!

@jaudiger
Copy link
Contributor Author

@kylewlacy I just pushed a rebase on top of main branch to resolve merge conflicts due to similar commits between this PR and main.

and it didn't seem to speed up or slow down things noticeably (~7 seconds before and after this change)

I wanted to also get some figures from the CI on brioche-packages to compare. As for now it takes around 15s to check the formatting of files of each package, one by one.

Copy link
Member

@kylewlacy kylewlacy left a comment

Choose a reason for hiding this comment

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

LGTM

@kylewlacy kylewlacy merged commit 1a5c04e into brioche-dev:main Oct 10, 2025
10 checks passed
@jaudiger jaudiger deleted the update-format-command branch October 11, 2025 06:19
@jaudiger
Copy link
Contributor Author

I wanted to also get some figures from the CI on brioche-packages to compare. As for now it takes around 15s to check the formatting of files of each package, one by one.

Before:
image

After:
image

Same time, just better output 😁

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.

2 participants