feat(mono): added merge checker#1367
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
benjamin-747
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds a merge checker system to validate merge requests before they are merged. The system introduces configurable checks that can be run on MRs, starting with an MR sync checker to detect merge conflicts.
- Added database schema for storing check configurations and results
- Implemented a checker registry system with a pluggable architecture
- Created MR sync checker to validate merge request synchronization status
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| mono/src/api/api_router.rs | Modified get_latest_commit to handle monorepo paths differently |
| jupiter/src/storage/mr_storage.rs | Added methods for updating MR hashes and managing check configurations/results |
| jupiter/src/model/mr_dto.rs | Added MrInfoDto struct for passing MR information to checkers |
| jupiter/src/migration/ | Added database migration and tables for check configurations and results |
| jupiter/callisto/src/ | Added enum definitions and entity models for check types and results |
| ceres/src/merge_checker/ | Implemented the checker registry and MR sync checker |
| ceres/src/pack/monorepo.rs | Integrated merge checking into the MR workflow |
| ceres/src/api_service/ | Minor formatting cleanup |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ]) | ||
| .to_owned(), | ||
| ) | ||
| .do_nothing() |
There was a problem hiding this comment.
The .do_nothing() call conflicts with the preceding .update_columns() call. When using on_conflict().update_columns(), you should not call .do_nothing() as it overrides the update behavior. Remove .do_nothing() to allow the conflict resolution to update the specified columns.
| .do_nothing() |
| #[async_trait] | ||
| impl Checker for MrSyncChecker { | ||
| async fn run(&self, params: &serde_json::Value) -> CheckResult { | ||
| let params = MrSyncParams::from_value(params).expect("parse params err"); |
There was a problem hiding this comment.
The error message "parse params err" is not descriptive. Consider using a more helpful message like "Failed to parse MR sync parameters" to help with debugging.
| let params = MrSyncParams::from_value(params).expect("parse params err"); | |
| let params = MrSyncParams::from_value(params).expect("Failed to parse MR sync parameters"); |
| .mr_storage() | ||
| .get_mr(link) | ||
| .await? | ||
| .expect("MR Not Found"); |
There was a problem hiding this comment.
The error message "MR Not Found" should be more descriptive and follow consistent formatting. Consider using "Failed to find MR" or a similar message that provides more context.
| .expect("MR Not Found"); | |
| .expect(&format!("Failed to find MR for link: {}", link)); |
No description provided.