Conversation
There was a problem hiding this comment.
Pull Request Overview
Implements automatic pull request creation for broken link fixes by persisting check results in the database and orchestrating git operations and GitHub API calls.
- Added branching, staging, commit, and push helpers to
RepoManager - Introduced
PullRequestGeneratorto fetch DB results, apply fixes, and create PR via GitHub API - Added database migrations for
repoandcheck_resulttables, updated module exports, and dependency
Reviewed Changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rook/src/git/repo.rs | Extended RepoManager with branch/checkout/add/commit/push helpers and added TempDirGuard::get_path |
| rook/src/git/pr_generator.rs | New PullRequestGenerator for DB-driven PR creation, with end-to-end tests |
| rook/src/git/mod.rs | Exported the new pr_generator module |
| rook/migrations/20250707131919_create_check_result_table.sql | Created repo and check_result tables for storing check results |
| rook/Cargo.toml | Added thiserror dependency for error handling |
Files not reviewed (2)
- rook/.sqlx/query-bdbac467ae90075b836db2aec5729c159c1e96d4cf691b0f6b4b51b2361cf710.json: Language not supported
- rook/.sqlx/query-cc44dadc799019ce0c67bf4ebf1b47f2a0e6ec4ce623e768afe1c25db025cabd.json: Language not supported
| fn get_repo_url(&self) -> Result<String, PrError> { | ||
| let repo_path = self.repo_manager.get_repo_path(); | ||
|
|
||
| let path_components: Vec<&str> = repo_path.to_str().unwrap().split('/').collect(); |
There was a problem hiding this comment.
Building the GitHub API URL from the local filesystem path will produce an incorrect owner/repo. Instead, retrieve the remote origin URL or accept the repository URL as an input parameter for accurate API endpoint construction.
| let repo_manager = RepoManager::clone_repo(TEST_REPO_URL, Some("main")).unwrap(); | ||
|
|
||
| // Create initial commit if needed | ||
| if repo_manager.has_uncommitted_changes().unwrap() { | ||
| repo_manager.add_all().await.unwrap(); | ||
| repo_manager | ||
| .commit("Test commit", "Test User", "test@example.com") | ||
| .await | ||
| .unwrap(); | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] These tests depend on an external GitHub repository (TEST_REPO_URL), which makes them non-hermetic and prone to network flakiness. Consider using a local temporary repository (e.g., via TempDirGuard) to isolate and speed up tests.
| let repo_manager = RepoManager::clone_repo(TEST_REPO_URL, Some("main")).unwrap(); | |
| // Create initial commit if needed | |
| if repo_manager.has_uncommitted_changes().unwrap() { | |
| repo_manager.add_all().await.unwrap(); | |
| repo_manager | |
| .commit("Test commit", "Test User", "test@example.com") | |
| .await | |
| .unwrap(); | |
| } | |
| // Create a temporary directory for the repository | |
| let temp_dir = TempDirGuard::new(env::temp_dir().join("test_repo")).unwrap(); | |
| let repo_path = temp_dir.get_path(); | |
| // Initialize a new Git repository in the temporary directory | |
| let repo = Repository::init(repo_path).unwrap(); | |
| // Create an initial commit | |
| let signature = Signature::now("Test User", "test@example.com").unwrap(); | |
| let tree_id = { | |
| let mut index = repo.index().unwrap(); | |
| index.write_tree().unwrap() | |
| }; | |
| let tree = repo.find_tree(tree_id).unwrap(); | |
| repo.commit( | |
| Some("HEAD"), | |
| &signature, | |
| &signature, | |
| "Initial commit", | |
| &tree, | |
| &[], | |
| ) | |
| .unwrap(); | |
| // Create a RepoManager instance for the local repository | |
| let repo_manager = RepoManager::from_path(repo_path).unwrap(); |
There was a problem hiding this comment.
굿 테스트를 작성할 다음 PR에서 작업해볼게욥
♟️ What’s this PR about?
파일을 수정하고 변경 사항에 대한 Pull Request를 생성하는 기능을 구현하였습니다.
PR 생성 요청 흐름을 어떻게 가져가는게 아름다울지 고민해보았는데,
일단 현 상황은 실시간으로 링크 체크를 확인할 수 있고(sse), 또 매일 메일로도 링크 체크 결과를 확인할 수 있도록 구현이 된 상태입니다. 두 가지 방식으로 작업이 끝난 다음 변경해야 하는 링크가 발견됐을 때, 이를 PR 생성기한테 전해줘야 하는데, 그냥 바로 전해주게 된다면 코드 구현이 꼬일거 같다는 생각이 들었습니다.
그래서 두 가지 방식 모두 작업이 끝난 다음 결과를 DB에 저장하고, PR 생성 요청이 오면 DB에서 저장된 결과가 있는지 확인하고, 결과가 없다면 오류를, 결과가 있다면 그 결과를 가지고 PR을 생성하는 흐름으로 구현을 하였습니다.
비동기적이고, 사용자가 PR 생성을 하고 싶지 않는 것에 대한 핸들링도 쉽다는 장점이 있습니다.
아직 테스트가 작성되지 않았는데, 테스트에서 DB를 다뤄야 하기에 테스트의 볼륨이 조금 커질 거 같아서 테스트 작성은 이 PR 작성하지 않았고 한번 끊어서 가려고 합니다.
🔗 Related Issues / PRs
part of #43 #145