Conversation
…ilerplate stubs from test MockApis
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBacklogApi trait in Changes
Sequence Diagram(s)sequenceDiagram
participant Cmd as Caller (command)
participant Client as BacklogClient
participant Trait as BacklogApi trait (default)
participant Impl as Concrete impl (e.g., MockApi / HTTP client)
Cmd->>Client: call Client.get_space()
Note right of Client: inherent impl forwards
Client->>Trait: calls self.get_space()
alt Concrete impl overrides
Trait->>Impl: dispatch to Impl.get_space()
Impl-->>Trait: Result<Space>
Trait-->>Client: return result
Client-->>Cmd: return result
else No override (uses trait default)
Trait-->>Trait: default body executes (unimplemented!() or other)
Trait-->>Client: panic / unimplemented
Client-->>Cmd: panic / error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors the BacklogApi trait to provide default unimplemented!() method bodies, allowing test mocks to implement only the API calls they actually exercise, and removes large amounts of stub-only boilerplate from command tests.
Changes:
- Added default
unimplemented!()bodies to allBacklogApitrait methods. - Removed stub-only
unimplemented!()method implementations from many testMockApiimpls, along with now-unneeded imports. - Kept
BacklogClient’sBacklogApiimplementation overriding all methods with real behavior.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/api/mod.rs | Adds default unimplemented!() bodies to BacklogApi methods to reduce test mock boilerplate. |
| src/cmd/auth.rs | Simplifies test MockApi impl by removing unused trait-method stubs. |
| src/cmd/wiki/update.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/wiki/show.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/wiki/list.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/wiki/history.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/wiki/delete.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/wiki/create.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/wiki/attachment/list.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/user/show.rs | Removes stub-only mock methods from tests. |
| src/cmd/user/list.rs | Removes stub-only mock methods from tests. |
| src/cmd/team/show.rs | Removes stub-only mock methods from tests. |
| src/cmd/space/update_notification.rs | Removes stub-only mock methods from tests. |
| src/cmd/space/show.rs | Removes stub-only mock methods from tests. |
| src/cmd/space/notification.rs | Removes stub-only mock methods from tests. |
| src/cmd/space/licence.rs | Removes stub-only mock methods from tests. |
| src/cmd/space/disk_usage.rs | Removes stub-only mock methods from tests. |
| src/cmd/project/version.rs | Removes stub-only mock methods from tests. |
| src/cmd/project/user.rs | Removes stub-only mock methods from tests. |
| src/cmd/project/status.rs | Removes stub-only mock methods from tests. |
| src/cmd/project/show.rs | Removes stub-only mock methods from tests. |
| src/cmd/project/list.rs | Removes stub-only mock methods from tests. |
| src/cmd/project/issue_type.rs | Removes stub-only mock methods from tests. |
| src/cmd/project/disk_usage.rs | Removes stub-only mock methods from tests. |
| src/cmd/project/category.rs | Removes stub-only mock methods from tests. |
| src/cmd/notification/reset_unread.rs | Removes stub-only mock methods/imports, leaving only the tested API call. |
| src/cmd/notification/read.rs | Removes stub-only mock methods/imports, leaving only the tested API call. |
| src/cmd/notification/count.rs | Removes stub-only mock methods/imports, leaving only the tested API call. |
| src/cmd/issue/update.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/show.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/list.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/delete.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/create.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/count.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/comment/update.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/comment/list.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/comment/delete.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/comment/add.rs | Removes stub-only mock methods and trims test imports. |
| src/cmd/issue/attachment/list.rs | Removes stub-only mock methods and trims test imports. |
You can also share your feedback on Copilot code review. Take the survey.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/developing/references/patterns.md:
- Around line 45-51: The examples are inconsistent with the new parameterized
signature for get_my_resource; update all declarations and call sites to accept
and forward params. Specifically, change the trait/default stub for
get_my_resource to fn get_my_resource(&self, params: &[(String, String)]) ->
Result<MyResource>, update BacklogClient::get_my_resource implementation to
accept params and forward them (self.get_my_resource(params)), and update any
example calls (e.g., api.get_my_resource() ) to pass a params value (e.g.,
api.get_my_resource(¶ms) or similar) so BacklogApi, BacklogClient and
example usage are consistent.
In `@src/api/mod.rs`:
- Around line 52-199: The default implementations in BacklogApi (e.g.,
get_space, get_myself, get_users, get_user, get_space_activities,
get_space_disk_usage, get_space_notification, get_projects, get_project,
get_project_activities, get_project_disk_usage, get_project_users,
get_project_statuses, get_project_issue_types, get_project_categories,
get_project_versions, get_issues, count_issues, get_issue, create_issue,
update_issue, delete_issue, get_issue_comments, add_issue_comment,
update_issue_comment, delete_issue_comment, get_issue_attachments, get_wikis,
get_wiki, create_wiki, update_wiki, delete_wiki, get_wiki_history,
get_wiki_attachments, get_teams, get_team, get_user_activities,
get_recently_viewed_issues, get_notifications, count_notifications,
read_notification, reset_unread_notifications, get_space_licence,
put_space_notification) currently call unimplemented!() which panics; replace
each panic with an anyhow-compatible error return (e.g., return
Err(anyhow::anyhow!("BacklogApi::<method_name> not implemented")) or use
anyhow::bail!) so the functions return a Result::Err instead of crashing; update
imports to include anyhow if missing and ensure the error message identifies the
specific method (use the method name symbol) for easier debugging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6dd45dc-f4ff-494b-a157-ef9965b54791
📒 Files selected for processing (2)
.claude/skills/developing/references/patterns.mdsrc/api/mod.rs
This was referenced Mar 15, 2026
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.
Checklist
mainSummary
unimplemented!()bodies to allBacklogApitrait methodsunimplemented!()stub implementations from testMockApistructs across 44 filesReason for change
Each
MockApiin tests had to implement all ~43 trait methods even when only 1–2 were relevant to the test. This made test files very long, hurt readability, and wasted context when AI reads the codebase.Changes
src/api/mod.rs: trait methods now have default{ unimplemented!() }bodies (with_-prefixed unused params)src/cmd/**/*.rs: removed all stub-only methods from testMockApi/MockApiCaptureimpls; only methods with meaningful logic remainNotes
impl BacklogApi for BacklogClientis unchanged — it still overrides every method with real HTTP calls