Skip to content

Fix build fail#916

Open
dvush wants to merge 2 commits intodevelopfrom
fix-test-build
Open

Fix build fail#916
dvush wants to merge 2 commits intodevelopfrom
fix-test-build

Conversation

@dvush
Copy link
Copy Markdown
Contributor

@dvush dvush commented Apr 13, 2026

📝 Summary

  1. add --all-targets to makefile checks (otherwise we miss linting tests and benches)
  2. fix compilation errors and warnings in tests

💡 Motivation and Context


✅ I have completed the following steps:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

@dvush dvush requested a review from ZanCorDX as a code owner April 13, 2026 14:54
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 13, 2026

Claude finished @dvush's task in 1m 15s —— View job


Review of PR #916

  • Gather context and read changed files
  • Review Makefile changes
  • Review benchmark file changes
  • Review deleted order_dumper.rs
  • Review block_orders/mod.rs changes
  • Post review summary

LGTM — this is a clean, straightforward fix. All four changes are correct and well-motivated:

Makefile (--all-targets): Adding --all-targets to both lint and fmt targets ensures clippy catches issues in tests, benches, and examples — not just lib/bin targets. The fmt target also correctly gains --workspace which it was previously missing (the lint target already had it).

Benchmark fix (txpool_fetcher.rs): The subscribe_to_txpool_with_blobs signature was updated upstream to require Arc<MempoolTxsDetector> as the third parameter. The benchmark now passes it correctly. No issues.

order_dumper.rs deletion: OrderDumper and OrderStoreAction are not referenced anywhere else in the codebase — confirmed via grep. The comment "Soon to be replaced my mockall" suggests it was already slated for removal. Clean delete.

block_orders/mod.rs: Simply removes the #[cfg(test)] mod order_dumper; declaration to match the file deletion. No other changes to this file.

No bugs, no security concerns, no performance issues.

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