Skip to content

Make unary_request internal-only (closes #200)#201

Merged
msk merged 2 commits into
mainfrom
octoaide/issue-200-2026-05-30T14-41-01
May 30, 2026
Merged

Make unary_request internal-only (closes #200)#201
msk merged 2 commits into
mainfrom
octoaide/issue-200-2026-05-30T14-41-01

Conversation

@octoaide

@octoaide octoaide Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

This PR removes unary_request from the public API and makes it
crate-internal (pub(crate)). The function is no longer intended for
external use and internal callers continue to use crate::unary_request
without changes.

Changes

  • src/lib.rs: change unary_request visibility to pub(crate)
  • CHANGELOG.md: document removal under [Unreleased] → Removed

Why
Reducing the public API surface clarifies the supported interfaces and
encourages use of the typed helpers (for example, client::Connection and
server::Connection) instead of a raw unary_request entrypoint.

Verification

  • cargo test --all-features — 200 tests passed
  • cargo clippy --tests --all-features — clean

Migration
If any downstream crate used review_protocol::unary_request directly,
switch to the higher-level APIs (client::Connection, server::Connection,
etc.).

See also: #200

Closes #200

@octoaide octoaide Bot requested a review from msk May 30, 2026 14:42
@codecov

codecov Bot commented May 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.68%. Comparing base (b2f4e12) to head (8963ff2).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #201      +/-   ##
==========================================
+ Coverage   85.61%   85.68%   +0.06%     
==========================================
  Files          15       15              
  Lines        6481     6481              
==========================================
+ Hits         5549     5553       +4     
+ Misses        932      928       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

unary_request was compiled for the server feature but only used by
the client API and by server tests, causing Clippy to report the
function as unused in non-test server builds.

Tighten the cfg to only build unary_request when the client feature
is enabled or when running tests with the server feature:

#[cfg(any(feature = "client", all(test, feature = "server")))]

Verified with clippy and cargo test. Change in src/lib.rs.
@octoaide

octoaide Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

I reviewed the Codecov report (informational — coverage is unchanged and all modified lines are covered) and investigated the CI failure from the check job.

What I found

  • The failing Clippy error was: function unary_request is never used. It happened because unary_request was being compiled any time the server feature was enabled, but in non-test server builds that function is only used by client code and by server tests — so it was legitimately unused in some build combinations.
  • Making the function pub(crate) didn’t solve the build combinations problem because it was still being compiled in contexts that don’t call it.

What I changed

  • I tightened the conditional compilation for unary_request so it is only compiled when it’s actually needed:
    #[cfg(any(feature = "client", all(test, feature = "server")))]
    This ensures the function is built for the production client API, and also when running server tests (which exercise unary requests), but not for normal server builds that don’t call it.

Verification

  • I committed and pushed the change.
  • I ran cargo clippy --tests for: no features, client, server, and --all-features — all clean.
  • I ran cargo test --all-features — all tests passed.

Result

  • The Clippy error is resolved and Codecov remains unchanged (coverage still OK). Everything required by the feedback has been addressed.

Thanks for the pointer — let me know if you want the cfg applied differently or prefer a comment explaining why the gate is narrow.

@msk msk merged commit 611db35 into main May 30, 2026
22 checks passed
@msk msk deleted the octoaide/issue-200-2026-05-30T14-41-01 branch May 30, 2026 14:51
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.

Remove unary_request

1 participant