Add REVIEW.md with PR #540 review responses and changelog

Author: satsfy

REVIEW.md

@@ -0,0 +1,93 @@
+# PR #540 Review Responses
+
+## Comment 1 — `is_https()`: replace `#[allow(dead_code)]` with cfg-gate
+**Reviewer:** tnull
+**File:** `bitreq/src/url.rs:386`
+**Request:** Apply `#[cfg(all(feature = "proxy", feature = "std"))]` instead of `#[allow(dead_code)]`.
+
+**Response:** Agree with the spirit — `allow(dead_code)` was the wrong approach and a cfg-gate
+is better. However, `is_https()` is used not only under `cfg(all(feature = "proxy", feature = "std"))` but also under `cfg(feature = "std")` in `ConnectionParams::from_request` (request.rs). The narrower gate the reviewer suggested would break that usage. Applied `#[cfg(feature = "std")]` instead, which covers both call-sites and the tests (which are also `cfg(test, feature = "std")`).
+
+**Change:** Replaced `#[allow(dead_code)]` with `#[cfg(feature = "std")]` on `is_https()`.
+
+---
+
+## Comment 2 — `path_and_query()`: replace `#[allow(dead_code)]` with cfg-gate
+**Reviewer:** tnull
+**File:** `bitreq/src/url.rs:405`
+**Request:** cfg-gate on `std` rather than allowing dead code.
+
+**Response:** Fully agree. `path_and_query()` is only called from `get_http_head()` (gated on `std`) and from tests (gated on `cfg(all(test, feature = "std"))`).
+
+**Change:** Replaced `#[allow(dead_code)]` with `#[cfg(feature = "std")]` on `path_and_query()`.
+
+---
+
+## Comment 3 — `webpki-roots` cfg rename: expand why it's necessary
+**Reviewer:** tnull
+**File:** `bitreq/src/connection/rustls_stream.rs:25`
+**Request:** Explain why `rustls-webpki` was renamed to `webpki-roots`.
+
+**Response:** This is a bug-fix, not a behavior change. The import
+`use webpki_roots::TLS_SERVER_ROOTS;` comes from the `webpki-roots` crate (dependency name
+`webpki-roots` in Cargo.toml). The old cfg `feature = "rustls-webpki"` referenced a *different*
+dependency (`rustls-webpki`, the verification library). Since `https-rustls` enables both
+`webpki-roots` and `rustls-webpki` features, the gate happened to work in practice, but it was
+checking the wrong feature. The rename makes the gate match the crate that provides the symbol.
+
+No code change needed — this is an explanation only.
+
+---
+
+## Comment 4 — DRY up duplicate `use super::{AsyncHttpStream, AsyncTcpStream}`
+**Reviewer:** tnull
+**File:** `bitreq/src/connection/rustls_stream.rs:36`
+**Request:** Deduplicate the two identical import lines with different cfg gates.
+
+**Response:** Agree. Merged the two cfg-gated imports into a single import using `any()`:
+```rust
+#[cfg(all(
+    feature = "async",
+    any(
+        feature = "tokio-rustls",
+        all(feature = "native-tls", not(feature = "rustls"), feature = "tokio-native-tls")
+    )
+))]
+use super::{AsyncHttpStream, AsyncTcpStream};
+```
+This factors out the common `feature = "async"` requirement and uses `any()` for the two
+backend alternatives.
+
+**Change:** Replaced two duplicate imports with a single `any()`-based cfg gate.
+
+---
+
+## Comment 5 — Dropping `msrv` from `clippy.toml`
+**Reviewer:** tnull
+**File:** `clippy.toml:1`
+**Request:** Why remove the msrv? Will clippy still respect the MSRV?
+
+**Response:** Since Rust 1.74, clippy reads `rust-version` from `Cargo.toml` automatically, so
+the explicit `clippy.toml` setting is technically redundant (all crates in the workspace already
+declare `rust-version = "1.75.0"`). However, keeping it doesn't hurt and serves as a clear,
+explicit safety net. Restored it to avoid any ambiguity.
+
+**Change:** Restored `msrv = "1.75.0"` in `clippy.toml`.
+
+---
+
+## Comment 6 — Where is the `msrv` toolchain defined?
+**Reviewer:** tnull
+**File:** `.github/workflows/rust.yaml:31`
+**Request:** Where is the msrv toolchain defined? Concern about per-crate MSRV divergence.
+
+**Response:** The MSRV toolchain is defined in `Cargo.toml` at `[workspace.metadata.rbmt.toolchains] msrv = "1.75.0"`. `cargo rbmt` reads this value and installs/selects that toolchain when `--toolchain msrv` is passed. Currently all crates in the workspace share the same MSRV (1.75.0), so a single workspace-level value is correct. If crate MSRVs diverge in the future (e.g. bitreq's TLS deps bump MSRV), rbmt supports per-crate MSRV overrides via `[package.metadata.rbmt.toolchains]` — but there's no need for that yet. No code change for this item.
+
+---
+
+## Changelog
+
+1. `bitreq/src/url.rs` — `is_https()`: `#[allow(dead_code)]` → `#[cfg(feature = "std")]`
+2. `bitreq/src/url.rs` — `path_and_query()`: `#[allow(dead_code)]` → `#[cfg(feature = "std")]`
+3. `bitreq/src/connection/rustls_stream.rs` — DRY: merged two duplicate `use super::{AsyncHttpStream, AsyncTcpStream}` imports into one with an `any()` cfg gate
+4. `clippy.toml` — Restored `msrv = "1.75.0"`