Skip to content

tooling: Speed up bin/fmt (from 5s to 1.3s on my machine)#35393

Open
def- wants to merge 1 commit intoMaterializeInc:mainfrom
def-:pr-fmt-speedup
Open

tooling: Speed up bin/fmt (from 5s to 1.3s on my machine)#35393
def- wants to merge 1 commit intoMaterializeInc:mainfrom
def-:pr-fmt-speedup

Conversation

@def-
Copy link
Contributor

@def- def- commented Mar 10, 2026 •
edited
Loading

Before and after:

Screen.Recording.2026-03-10.at.14.55.46.mov

@github-actions
Copy link

github-actions bot commented Mar 10, 2026

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

  • Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
  • Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
  • Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

@def- def- force-pushed the pr-fmt-speedup branch 3 times, most recently from 2909437 to f7b7a14 Compare March 10, 2026 13:53
@def- def- changed the title tooling: Speed up bin/fmt from 6s to 1.3s tooling: Speed up bin/fmt from 5s to 1.3s Mar 10, 2026
@def- def- marked this pull request as ready for review March 10, 2026 13:57
@def- def- requested a review from a team as a code owner March 10, 2026 13:57
@def- def- requested a review from bosconi March 10, 2026 13:57
@def- def- changed the title tooling: Speed up bin/fmt from 5s to 1.3s tooling: Speed up bin/fmt (from 5s to 1.3s on my machine) Mar 10, 2026
@def- def- requested a review from antiguru March 10, 2026 16:20
antiguru
antiguru reviewed Mar 10, 2026
View reviewed changes
Copy link
Member

@antiguru antiguru left a comment

Choose a reason for hiding this comment

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

Looks good, but there seems to be an unrelated change in the PR.

src/testdrive/src/action/kinesis.rs Outdated
Copy link
Member

@antiguru antiguru Mar 10, 2026

Choose a reason for hiding this comment

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

Is this intended?

Copy link
Contributor Author

@def- def- Mar 10, 2026

Choose a reason for hiding this comment

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

Yes, it was causing rustfmt to complain, the file is actually unused and we just haven't noticed.

@def- def- requested review from antiguru and bobbyiliev March 10, 2026 17:04
@def- def- enabled auto-merge (squash) March 11, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bosconi bosconi Awaiting requested review from bosconi

@antiguru antiguru Awaiting requested review from antiguru

@bobbyiliev bobbyiliev Awaiting requested review from bobbyiliev

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@def- @antiguru