Skip to content

chore: cargo fmt new code in 3472c74 + my test#11

Closed
doctatortot wants to merge 1 commit into
ourmem:mainfrom
doctatortot:upstream-clean/fmt-followup
Closed

chore: cargo fmt new code in 3472c74 + my test#11
doctatortot wants to merge 1 commit into
ourmem:mainfrom
doctatortot:upstream-clean/fmt-followup

Conversation

@doctatortot
Copy link
Copy Markdown
Contributor

Tiny fmt sweep — cargo fmt --all on top of upstream main. Surfaces two drifts that landed alongside your larger cleanup in 508e9e6:

Pure fmt, no logic touched. cargo fmt -- --check passes after this; CI will actually run the 379 tests instead of bailing at the fmt step.

Worth thinking about: how to prevent future fmt drift

Now that you've done the big cleanup, fmt drift is going to keep biting both of us at the same rate until something enforces it pre-commit. Three options in increasing scope, all easy to add — happy to do whichever one you prefer in a follow-up PR (or nothing if you'd rather just keep an eye on it):

  1. Document in CONTRIBUTING.md — "run cargo fmt before committing." Cheapest; relies on discipline.
  2. .githooks/pre-commit + a one-line setup note (git config core.hooksPath .githooks). Hook runs cargo fmt --check on staged files; rejects the commit if dirty. Contributors opt in once per clone. Common Rust-project pattern.
  3. cargo-husky as a dev-dep — auto-installs the hook on first cargo build. Zero contributor setup, but adds a transitive dev-dep.

Let me know which (if any) and I'll open a follow-up.

🤖 Generated with Claude Code

Tiny fmt sweep — `cargo fmt --all` on top of upstream main. Surfaces
two formatting drifts that landed alongside the larger cleanup
(508e9e6):

- 5 long `format!` chains in api/mod.rs from the percent-encoding
  test fix (3472c74), now wrapped to per-line args
- One `.iter().find().unwrap()` chain in pipeline.rs from the
  test_rrf_normalize_weak_top_not_inflated test I added in ourmem#9,
  now broken across lines

Pure fmt — no logic touched. `cargo fmt -- --check` now passes
again, which means CI runs all 379 tests instead of bailing at the
fmt step.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
doctatortot added a commit to doctatortot/omem that referenced this pull request May 19, 2026
Same content as ourmem#11 — folding into this PR so a single merge fixes
everything currently broken in CI.

- 5 long `format!` chains in api/mod.rs from the percent-encoding fix
  in 3472c74 — now wrapped to per-line args
- One `.iter().find().unwrap()` chain in pipeline.rs from the
  `test_rrf_normalize_weak_top_not_inflated` test added in ourmem#9

Pure fmt, no logic.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@doctatortot
Copy link
Copy Markdown
Contributor Author

Closing as superseded by #16, which folds this exact fmt fix into the CI-protoc fix so a single merge restores green CI. No content changes.

yhyyz pushed a commit that referenced this pull request May 20, 2026
`lance-encoding`'s build.rs invokes `protoc` to generate prost bindings.
Without it `cargo clippy` and `cargo test` fail at compile time with
"Could not find `protoc`. If `protoc` is installed, try setting the
`PROTOC` environment variable..."

This was masked previously because CI bailed at the `cargo fmt --check`
step (229 sites of fmt drift across upstream main). After the fmt
cleanup in 508e9e6, PRs now reach the clippy step and surface this dep.
Three currently-open PRs (#11 #12 #14) hit this on their last CI runs.

Adding `apt-get install protobuf-compiler` before the rust toolchain
step matches the standard pattern for Rust projects with prost-build
deps (see lancedb, datafusion, deltalake CI configs).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

1 participant