Skip to content

apollo_network: delete dead code - #15069

Open
asaf-sw wants to merge 1 commit into
mainfrom
code_slayer/remove_dead_code_in_apollo_network_peer_manager
Open

apollo_network: delete dead code#15069
asaf-sw wants to merge 1 commit into
mainfrom
code_slayer/remove_dead_code_in_apollo_network_peer_manager

Conversation

@asaf-sw

@asaf-sw asaf-sw commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

What

Deletes the unused simulate_connection_established test helper in peer_whitelist_test.rs, along with the #[allow(dead_code)] that masked it and the imports (ConnectedPoint, ConnectionEstablished, FromSwarm) that become unused once it is gone.

The helper has no callers anywhere in the file. (It is a distinct function from the identically named, still-live helper in peer_manager/test.rs, which is untouched.) Removing its #[allow(dead_code)] makes the compiler report it as never used in both the default and --tests configurations; after deletion the crate builds with zero dead-code/unused warnings in both.

Scope change from the first revision

An earlier revision also removed PeerManager::report_session + PeerManagerError::NoSuchSession. Per @matanl-starkware's review, that code is intentionally kept for upcoming peer-reporting / decentralization work, so it has been fully restored (including its #[allow(dead_code)]). Only the unrelated dead test helper remains in this PR.

Verification (RUSTC_WRAPPER unset, CARGO_INCREMENTAL=0)

  • scripts/rust_fmt.sh — clean
  • cargo build -p apollo_network — zero warnings
  • cargo build -p apollo_network --tests — zero warnings
  • cargo clippy -p apollo_network --all-targets — clean
  • SEED=0 cargo test -p apollo_network — 126 + 25 passed, 0 failed

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@asaf-sw
asaf-sw force-pushed the code_slayer/remove_dead_code_in_apollo_network_peer_manager branch from fb1398b to 4171d43 Compare August 29, 2026 06:04

@matanl-starkware matanl-starkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@matanl-starkware made 1 comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on asaf-sw).


-- commits line 2 at r1:
I prefer to keep this code. Reporting peers is something we should consider adding as we move towards decentralization.

Code quote:

apollo_network: delete dead code

Delete the unused `simulate_connection_established` test helper in
`peer_whitelist_test.rs`, along with the `#[allow(dead_code)]` that masked it
and the imports (`ConnectedPoint`, `ConnectionEstablished`, `FromSwarm`) that
become unused once it is gone.

The helper has no callers anywhere in the file (it is a distinct function from
the identically named, live helper in `peer_manager/test.rs`). Removing its
`#[allow(dead_code)]` makes the compiler report it as never used in both the
default and `--tests` configurations; after deletion the crate builds with zero
dead-code/unused warnings in both configurations.

Note: an earlier revision of this PR also removed `PeerManager::report_session`
and `PeerManagerError::NoSuchSession`; per reviewer feedback that code is
intentionally kept for upcoming peer-reporting/decentralization work, so it was
restored and only the unrelated dead test helper remains.

Verification (RUSTC_WRAPPER unset, CARGO_INCREMENTAL=0):
- scripts/rust_fmt.sh — clean
- cargo build -p apollo_network — zero warnings
- cargo build -p apollo_network --tests — zero warnings
- cargo clippy -p apollo_network --all-targets — clean
- SEED=0 cargo test -p apollo_network — 126 + 25 passed, 0 failed
@asaf-sw
asaf-sw force-pushed the code_slayer/remove_dead_code_in_apollo_network_peer_manager branch from 4171d43 to 35a4d30 Compare August 30, 2026 07:18

asaf-sw commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @matanl-starkware — makes sense. I've restored PeerManager::report_session and PeerManagerError::NoSuchSession (with their #[allow(dead_code)]) so they're kept for the upcoming peer-reporting / decentralization work.

The PR now only removes the unrelated, genuinely-unused simulate_connection_established helper in peer_whitelist_test.rs (it has no callers in that file, and is a separate function from the live one in peer_manager/test.rs). If you'd rather keep that too, feel free to close this — no objection.


Generated by Claude Code

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.

4 participants