config: prefix the remaining setters with with_ and hide Limits' fields - #261
Merged
Merged
Conversation
…elds
`Limits::{max_request_body_size, max_message_size, element_memory_limit}`
and `CompressionPolicy::min_size` were the config setters that carried no
prefix, against the `with_` convention the other configuration setters
use. `Limits` also exposed its three fields publicly under the same names
as its setters, so there were two ways to set each value while
`#[non_exhaustive]` ruled out the struct-update syntax that public fields
would otherwise buy.
The setters are renamed and the fields are private, with accessors
following the `ClientConfig` shape: `with_x` to set, `x()` to read, each
cross-referencing the other. `CompressionPolicy` needed the rename only -
its fields were already private - and gains `min_size()`, which the rename
freed up and which it had no read path for before.
Every migration path is a compile error rather than a silent change: the
old setter name now resolves to a no-argument accessor, so passing a value
is an arity error, and a bare field read hits a private field.
Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
iainmcgin
force-pushed
the
refactor/limits-setter-naming
branch
from
July 24, 2026 23:23
e971201 to
91ce727
Compare
iainmcgin
marked this pull request as ready for review
August 1, 2026 21:16
iainmcgin
enabled auto-merge
August 1, 2026 21:16
asacamano
approved these changes
Aug 21, 2026
iainmcgin
added a commit
to rkk-ant/connect-rust
that referenced
this pull request
Aug 22, 2026
…migration EncodedBody becomes the Dispatcher streaming contract with this change and has not shipped yet, so seal its variants now rather than after a release; callers already go through segments()/into_contiguous(). Put the Bytes -> EncodedBody conversion for hand-written dispatchers on EncodedStream's rustdoc as a compiled example rather than only in the changelog, and say on Response::compress and in the guide's view-body section that a compressed response flattens the segmented encode (the default for >1 KiB responses to a gzip-advertising client), so a large-field stream wants compress(false). Reword the fragment: message bytes are unchanged but HTTP frame boundaries move, and the setter is with_min_size. Includes the with_min_size rename in one test from rebasing over connectrpc#261. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #247
Renames the four config setters that carried no prefix —
Limits::{max_request_body_size, max_message_size, element_memory_limit}andCompressionPolicy::min_size— towith_-prefixed forms, and makes the threeLimitsfields private with plain accessors.Breaking, so it wants the 0.9.0 window, and taking both halves at once means the type is only broken once.
The naming was the smaller half. Every other config type in the crate uses
with_, so a user who has learned it onClientConfigorServerreaches forlimits.with_max_message_size(..), gets method-not-found, and has to read the source. The overlap with the public fields was the sharper problem: field and method shared an identifier, so there were two ways to set each value, while#[non_exhaustive](whichLimitsalso gains in 0.9.0) rules out the struct-update syntax that public fields would otherwise buy.ClientConfig— private fields,with_setters, plain accessors — is the shape this now follows, including the doc convention of cross-linking each setter and accessor to its partner.The rename is not a silent break in either direction. A call site that kept the old setter name now resolves to a no-argument accessor, so passing a value is an arity error; a bare field read hits a private field. Every migration path is a compile error, which is what you want.
Issue #247 as I filed it claimed there were exactly three unprefixed setters. There were four —
CompressionPolicy::min_sizeis the same shape and is called on the client side, so the issue's framing of these as "server config setters" was also wrong. Both corrected; the changelog fragment now claims only that these were the last config types whose setters were unprefixed, which is true.ClientConfig::json,protoandcompress_requestsremain unprefixed and are deliberately out of scope here.CompressionPolicyneeded the rename only — its fields were already private, and it has no read path to collide with the new name, so no accessor was added.Ten
Limitscall sites and sevenmin_sizesites are updated across the library, tests, conformance harness, rustdoc examples and the guide, along with theElementMemoryLimitExceedederror text, which names the setter an operator should raise. One pre-existing unreleased changelog fragment is edited because its Rust example called a renamed setter, and one stale guide reference to a method that never existed is corrected.One note on CI: two
handler::testselement-budget tests fail onmainright now, independently of this change — fixture rot from buffa 0.9.1, fixed by #239.