Resolvable reviews#1442
Conversation
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: Aid C. <57825561+AidCheng@users.noreply.github.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Signed-off-by: Aid C. <57825561+AidCheng@users.noreply.github.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the reviewer and review conversation system to improve the MR workflow by transitioning from ID-based to link-based operations and adding review resolution capabilities.
Key changes:
- Refactored reviewer schema to use
mr_linkandusernameinstead ofmr_idandcampsite_id - Added review resolution state tracking with a
resolvedfield in conversations - Enhanced review comment flow to automatically tag reviewer comments as
Reviewtype
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| mono/src/api/mr/mr_router.rs | Updated API endpoints to use new reviewer schema and added review resolution endpoint |
| mono/src/api/mr/model.rs | Modified payload structures to align with new data model |
| jupiter/src/storage/mr_reviewer_storage.rs | Refactored storage methods to use mr_link and username parameters |
| jupiter/src/storage/conversation_storage.rs | Added review resolution state management functionality |
| jupiter/src/migration/m20250910_153212_add_username_to_reviewer.rs | Database migration for schema changes |
| jupiter/callisto/src/mega_mr_reviewer.rs | Updated entity model with new fields |
| jupiter/callisto/src/mega_conversation.rs | Added resolved field to conversation model |
| jupiter/callisto/src/entity_ext/mega_conversation.rs | Enhanced conversation creation with resolution state |
| ceres/src/merge_checker/code_review_checker.rs | Updated to use new reviewer storage interface |
Comments suppressed due to low confidence (1)
ceres/src/merge_checker/code_review_checker.rs:69
- The code references
reviewer.idbut the new reviewer model usesusernamefield. This should bereviewer.username.
let msg = format!("Reviewer {} has not approved the MR.\n", reviewer.id);
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pub struct Migration; | ||
|
|
||
| #[async_trait::async_trait] | ||
| #[async_trait::async_trait] |
There was a problem hiding this comment.
Duplicate #[async_trait::async_trait] attribute should be removed. Only one instance is needed.
| #[async_trait::async_trait] |
| e | ||
| })?; | ||
|
|
||
| Ok(true) |
There was a problem hiding this comment.
The is_reviewer method always returns Ok(true) regardless of whether a reviewer is found. It should return Ok(reviewer.is_some()) to correctly indicate if the user is a reviewer.
| tracing::error!("{}", e); | ||
| MegaError::with_message(format!("fail to update reviewer {}", reviewer.clone())) | ||
| MegaError::with_message(format!("fail to update reviewer {}", reviewer_username)) | ||
|
|
There was a problem hiding this comment.
[nitpick] Extra blank line between error message and closing bracket should be removed for consistency.
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Summary
This pull request introduces key improvements to the reviewer and review conversation system in the MR workflow:
mr_linkandusername; added timestamps.resolvedfield for tracking conversation status.mr_link.Review.