docs: record decision to keep the _DIMiddlewareFactory split#20
Merged
Conversation
An architecture review flagged _DIMiddlewareFactory as a possible shallow module. The deletion test says otherwise: it's the adapter to FastStream's middleware-construction contract, so collapsing it moves the container-binding complexity rather than concentrating it. Record the "leave alone" verdict with a revisit trigger so future reviews don't re-litigate it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
1 participant
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.
What
First entry in
planning/decisions/— records the architecture-review verdict to keep_DIMiddlewareFactoryas a distinct class rather than collapsing it into_DiMiddleware.Why
The review flagged the factory as a possible shallow module (a one-method class that only binds
di_containerand forwards viaParamSpec). The deletion test says leave it: the factory is the adapter to FastStream's middleware-construction contract —add_middlewareneeds a callable that returns aBaseMiddleware, so the container must be bound ahead of that deferred call. Collapsing into apartial/closure moves that binding complexity rather than concentrating it, and likely keeps the same# ty: ignore.Recording it with a revisit trigger (FastStream accepts a pre-bound instance, or the
ParamSpecforwarding becomes type-clean) means future reviews won't re-litigate it.Verification
just check-planning—planning: OK.just indexlists the decision under newest-first Decisions.Docs-only; no code or behaviour change.
🤖 Generated with Claude Code