docs(review): split code-design from code-quality - #129
Merged
Conversation
Add conventions/review/code-design.md — the design-judgment tier (cohesion, coupling, responsibility, abstraction fit, substitutability, pattern application) between architecture.md and code-quality.md. - Relocate the design-judgment bullets out of code-quality.md — god class, missing abstractions, Law of Demeter / Hollywood Principle, and duplicated-structure (rule of three) — into code-design.md, leaving code-quality.md to local design, shape, style, and authoring hygiene. - Add the Fowler design smells code-quality.md lacked (feature envy, shotgun surgery, divergent change, insider trading, refused bequest, speculative generality, middle man, temporary field, anemic domain object), the LSP/ISP bars, and pattern-misuse smells (premature patternization, singleton-as-global-state, needless indirection). - State the abstraction-fit tension explicitly: DIP pushes toward more interfaces, speculative generality and shallow modules toward fewer — resolve on need, not count. - Update the review README index; drop the moved Large Class cite. Smells verified against the Refactoring 2nd-ed catalog, GoF, Martin's SOLID, and Ousterhout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1x1KNkkjSe2CjbuTb4Gis
|
Claude finished @kyle-sexton's task in 0s —— View job I'll analyze this and get back to you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bf4f83ccb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… propagation The example (add a field → DTO, mapper, validator, migration, handlers) described required boundary/schema propagation, which reviewers could misread as a blockable smell. Reframe the trigger to scattered duplicated *logic* and add an explicit carve-out for necessary per-layer plumbing. Addresses the codex-connector finding on #129. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1x1KNkkjSe2CjbuTb4Gis
|
Claude finished @kyle-sexton's task in 0s —— View job I'll analyze this and get back to you. |
3 tasks
kyle-sexton
added a commit
that referenced
this pull request
Jul 16, 2026
## Summary New `conventions/review/multi-tenancy.md` — diff-time review bars for a multi-tenant (SaaS) system, organized around **horizontal authorization**: keeping an already-authenticated request confined to its own tenant. `security.md` owns the vertical boundary (authn, sanitization, injection); this file owns everything downstream of "we know who is calling." Criteria (all diff-anchored; a cross-tenant read/write defaults to **Critical**): - **Tenant-scoped data access** — missing tenant predicate; fetch-by-id without ownership; conditional per-tenant key selection. - **Tenant identity and trust** — client-supplied tenant used for scoping; enumerable cross-tenant identifiers (BOLA). - **Shared state and configuration** — cache key omits the tenant; global flag driving a tenant decision. - **Asynchronous work** — tenant context lost across the queue/job/event boundary. - **Fairness** — unbounded tenant-triggerable work with no per-tenant quota (noisy neighbor). - **Tenant lifecycle** — offboarding that misses a data store; provisioning defaults. - **Boundaries** section delimiting ownership vs `security.md` / `performance.md` / `concurrency.md` / `observability.md`. Registered in the review README (placed by `security.md`, away from the code-design row in #129 to avoid a README merge conflict between the two held PRs). ## Sourcing Verified against **AWS Well-Architected SaaS Lens** (isolation mindset), **OWASP** (API1:2023 Broken Object Level Authorization + Authorization Cheat Sheet), and the **Azure multitenant architecture** guides. Dropped non-diff-reviewable candidates (tenancy-model choice, capacity planning, cost metering) by design; per-tenant encryption kept as an explicitly conditional bar. ## Test plan - [x] Local `lefthook` pre-commit — typos, editorconfig, gitleaks, markdownlint pass - [ ] CI `lychee` will validate the source URLs - [ ] Held for your review (hybrid policy) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- 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.
Summary
Introduces the design-judgment review tier as its own slice, resolving the taxonomy question of where "code design" belongs relative to code-quality.
conventions/review/code-design.md— cohesion, coupling, responsibility assignment, abstraction fit, substitutability, and pattern application. Sits betweenarchitecture.md(cross-unit structure/contracts) andcode-quality.md(surface hygiene).code-quality.md(god class, missing abstractions, Law of Demeter / Hollywood, duplicated-structure/rule-of-three);code-quality.mdnow owns local design, shape, style, and authoring hygiene.architecture-and-design.md).Design decisions folded in
architecture-and-design.md's explicit allowance of within-aggregate/fluent chains; the cross-boundary case is already Law of Demeter) and Parallel Inheritance Hierarchies (removed in Refactoring 2nd ed; not single-diff-visible).Test plan
lefthookpre-commit — typos, editorconfig, gitleaks, markdownlint passlycheewill validate the new source URLs🤖 Generated with Claude Code