Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the issue where providing None as the branch caused the default branch not to be found by refactoring how branch data is handled.
- Updated RepoKey to use an Option for branch
- Removed local defaulting of branch in schedule functions and adjusted logging messages
- Modified git module functions to accept branch as an Option and updated branch checkout logic
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| rook/src/schedule/mod.rs | Converted branch to Option and removed forced defaulting of branch. |
| rook/src/git/mod.rs | Updated functions to handle branch as an Option and refactored checkout logic. |
Comments suppressed due to low confidence (3)
rook/src/schedule/mod.rs:24
- Removing the default branch conversion here shifts its responsibility to downstream functions. Verify that this behavior is consistent and that the git module correctly defaults to 'default' when branch is None.
let branch = branch.unwrap_or_else(|| "default".to_string());
rook/src/schedule/mod.rs:13
- The change to use an Option type for the branch field in RepoKey is a good design move for handling None values. Please ensure that all related components (e.g., hash map keys) correctly handle cases when branch is None.
branch: Option<String>,
rook/src/git/mod.rs:59
- The updated checkout_branch function now directly checks out the commit tree and sets the HEAD without creating a local tracking branch. Confirm that this behavior aligns with the intended branch tracking requirements.
repo.checkout_tree(commit.as_object(), None)?;
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.
♟️ What’s this PR about?
branch 이름으로 None을 주었을 때 default 브랜치를 찾지 못하던 문제를 해결하였습니다.
원인은 schedule 에서 지맘대로 None을 "default"로 바꾼다음 git 모듈에게 넘겨준게 문제였습니다.
schedule의 RepoKey 필드를 branch: String 에서 branch: Option으로 변경하고 코드 리팩터링을 하였습니다.
🔗 Related Issues / PRs
close: #76