chore(chat-cli): switch transport at runtime via --transport flag#95
chore(chat-cli): switch transport at runtime via --transport flag#95
Conversation
There was a problem hiding this comment.
Pull request overview
Updates chat-cli to compile both file + logos-delivery transports into a single binary and select the transport at runtime via a new --transport flag (defaulting to logos-delivery), replacing the prior build-time cfg split.
Changes:
- Add
--transport {logos-delivery|file}CLI flag and runtime dispatch to the selected transport. - Make both transport modules always available in the
chat-clicrate (nocfg(logos_delivery)gating). - Adjust build/docs/CI to reflect that linking
liblogosdeliveryis now required for buildingchat-cli.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| bin/chat-cli/src/transport.rs | Removes compile-time cfg gating so both transports are built. |
| bin/chat-cli/src/main.rs | Adds --transport flag and refactors startup into runtime dispatch + shared run() path. |
| bin/chat-cli/build.rs | Makes LOGOS_DELIVERY_LIB_DIR required and always links liblogosdelivery. |
| bin/chat-cli/README.md | Documents runtime transport selection and updates examples/options. |
| bin/chat-cli/Cargo.toml | Removes unexpected_cfgs lint config tied to removed cfg usage. |
| README.md | Updates top-level docs to mention runtime transport selection. |
| .github/workflows/ci.yml | Excludes chat-cli from toolchain-only build/test/clippy, relying on Nix smoketest for chat-cli verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Both file and logos-delivery transports are now compiled into a single binary and selected at runtime (default: logos-delivery), replacing the env-var-driven build-time cfg.
osmaczko
force-pushed
the
chore/chat-cli-runtime-transport-switch
branch
from
1bfcb19 to
9e5f557
Compare
| use transport::logos_delivery::{Config, Service}; | ||
|
|
||
| eprintln!("Starting logos-delivery node (preset={})...", cli.preset); | ||
| eprintln!("This may take a few seconds while connecting to the network."); |
There was a problem hiding this comment.
I'm wondering why use eprintln, not println or to the log file?
| let transport_dir = cli.data.join("transport"); | ||
| let (transport, inbound) = transport::file::FileTransport::new(&transport_dir) | ||
| .context("failed to create file transport")?; | ||
| run(transport, inbound, &cli) |
There was a problem hiding this comment.
[Dust] Managing two separate invocations is likely to cause bugs in the future. You could return a Box<dyn DeliveryService>; from the match/setup function. This means that the invocation site for all transports would be the same.
| @@ -26,7 +29,7 @@ jobs: | |||
| - uses: actions/checkout@v4 | |||
| - run: rustup update stable && rustup default stable | |||
| - run: rustup component add clippy | |||
| - run: cargo clippy --all-targets --all-features -- -D warnings | |||
| - run: cargo clippy --all-targets --all-features --workspace --exclude chat-cli -- -D warnings | |||
There was a problem hiding this comment.
These --exclude chat-cli's are telling me that chat-cli crate likely does not belong inside of this repo if needs to keep being handled differently.
Both file and logos-delivery transports are now compiled into a single binary and selected at runtime (default: logos-delivery), replacing the env-var-driven build-time cfg.
Motivation: #91 (comment)