Conversation
|
This change is part of the following stack: Change managed by git-spice. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #739 +/- ##
==========================================
+ Coverage 55.66% 55.69% +0.02%
==========================================
Files 126 126
Lines 13759 13763 +4
==========================================
+ Hits 7659 7665 +6
+ Misses 6100 6098 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
👀 Conductor: Awaiting ReviewerPR #739 ( Nudge posted by conductor pipeline sync. |
geoffjay
left a comment
There was a problem hiding this comment.
Review: refactor(common): extract ApiError::status_code helper
Stack position: issue-720 is directly on main with no parent PR
dependency, but git-spice reports (needs restack) — the branch is behind
main and must be rebased before the conductor can merge it.
Code quality: approved
The change is correct, focused, and well-tested. Specific notes:
status_code() method — the variant→status mapping is identical to the
old per-arm match in into_response. No behaviour change.
Internal case — the old code used e.to_string() (on the inner
anyhow::Error); the new code uses self.to_string(). Because Internal is
annotated #[error(transparent)], Display delegates to the inner error, so
the two produce identical strings. Correct.
into_response simplification — going from 7 match arms (each repeating
self.to_string()) to two lines is the right outcome. The match &self /
self.to_string() awkwardness (matching a reference to get the status then
calling to_string on the owner) is gone.
Tests — 7 tests, one per variant, all exercising status_code() directly.
anyhow::anyhow!("boom").into() for Internal is the idiomatic pattern.
Coverage is complete.
Scope — single file, single responsibility, no unrelated changes.
Action required before merge
Rebase onto main:
git-spice branch restack
git-spice branch submitOnce restacked, add merge-queue to trigger the conductor.
refactor(common): extract ApiError::status_code helper
refactor(common): extract ApiError::status_code helper from into_response (closes #720)