-
Notifications
You must be signed in to change notification settings - Fork 86
Remove common ancestor #948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v3_er
Are you sure you want to change the base?
Conversation
eyalk007
commented
Nov 3, 2025
- All tests passed. If this feature is not already covered by the tests, I added new tests.
- This pull request is on the dev branch.
- I used gofmt for formatting the code before submitting the pull request.
- Update documentation about new features / new supported technologies
| return getScanStatus(converted...) | ||
| } | ||
|
|
||
| func prepareTargetForScan(gitDetails utils.Git, scanDetails *utils.ScanDetails) (targetBranchWd string, cleanupTarget func() error, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont think it should be a function anymore, its called once and its has only two lines now
| if e := tryCheckoutToMostCommonAncestor(scanDetails, gitDetails.PullRequestDetails.Source.Name, target.Name, targetBranchWd, repoCloneUrl); e != nil { | ||
| log.Warn(fmt.Sprintf("Failed to get best common ancestor commit between source branch: %s and target branch: %s, defaulting to target branch commit. Error: %s", gitDetails.PullRequestDetails.Source.Name, target.Name, e.Error())) | ||
| } | ||
| // Download latest target branch (no common ancestor logic) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets delete comment, future frogbot developers should not be familiar with common ancestor logic
