feat: rebase pull request branch on checkout PR#674
Conversation
|
Report bugs in Issues The following are automatically added:
Available user actions:
Supported /retest check runs
Supported labels
|
WalkthroughThe pull request introduces a modification to the Changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
webhook_server_container/libs/github_api.py (2)
1645-1645: Added rebase operation may fail in some scenariosThis adds a rebase operation after checking out a specific branch or tag. While the implementation accomplishes the PR's goal of rebasing the PR branch on the base branch before running CI tests, there are a few concerns:
- There's no error handling if the rebase fails due to conflicts
- The rebase happens unconditionally whenever the
checkoutparameter is providedConsider adding error handling to gracefully handle rebase conflicts:
- run_command(f"{git_cmd} rebase {self.pull_request_branch}", log_prefix=self.log_prefix) + rc, out, err = run_command(f"{git_cmd} rebase {self.pull_request_branch}", log_prefix=self.log_prefix) + if not rc: + self.logger.warning(f"{self.log_prefix} Failed to rebase on {self.pull_request_branch}. Error: {err}") + # Consider whether to proceed or fail the operationAdditionally, you might want to make the rebase conditional based on context (e.g., only when working with PR branches rather than tags).
1615-1672: Consider updating docstring to reflect the rebasing behaviorThe method's context and responsibilities have changed with the addition of the rebase operation, but the docstring hasn't been updated to reflect this.
Update the docstring to clarify that when
checkoutis specified, the method will also attempt to rebase the checked-out branch on the PR's base branch:@contextlib.contextmanager def _prepare_cloned_repo_dir( self, clone_repo_dir: str, is_merged: bool = False, checkout: str = "", tag_name: str = "", ) -> Generator[None, None, None]: + """ + Prepare a cloned repository directory for operations. + + When a checkout branch or tag is specified, this method will also rebase it + on the PR's base branch to ensure it's up-to-date with latest changes. + + Args: + clone_repo_dir: Directory where the repo will be cloned + is_merged: Whether the PR has been merged + checkout: Branch or tag to checkout + tag_name: Tag name to checkout + + Yields: + None: Control is yielded to the caller for operations on the repo + """ git_cmd = f"git --work-tree={clone_repo_dir} --git-dir={clone_repo_dir}/.git"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webhook_server_container/libs/github_api.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: python-module-install
- GitHub Check: tox
- GitHub Check: build-container
- GitHub Check: pre-commit
🔇 Additional comments (1)
webhook_server_container/libs/github_api.py (1)
1642-1646:Details
✅ Verification successful
Verify the meaning of
self.pull_request_branchin this contextThe current implementation rebases onto
self.pull_request_branchafter checkout. Based on howpull_request_branchis initialized in the code (line 208:self.pull_request_branch = self.pull_request.base.ref), it appears this variable represents the base branch of the PR (likemainormaster), not the PR's feature branch.Let me confirm this understanding is correct by checking the PR branch construction:
🏁 Script executed:
#!/bin/bash # Verify how pull_request_branch is determined to ensure it's the base branch rg -A 3 "self.pull_request_branch = " webhook_server_container/libs/github_api.pyLength of output: 753
Confirmation: Rebasing onto the PR Base Branch is Correct
After verifying the assignments (e.g., usingself.pull_request_branch = self.pull_request.base.refandself.pull_request_branch = pull_request_data["base"]["ref"]), it’s clear thatself.pull_request_branchconsistently represents the PR’s base branch (likemainormaster). The rebase command is therefore intended to operate against the base branch after checkout. No change is needed.
…rebase-when-checkout-pr
|
/verified |
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
2 participants
When checkout PR to run CI tests we need to rebase on the PR base branch.
With this we always run against the updated code.
Summary by CodeRabbit