Skip to content

Conversation

@knst
Copy link
Collaborator

@knst knst commented Sep 1, 2023

Issue being fixed or feature implemented

Seems as old command git merge-base --fork-point doesn't work for some cases of tree structure

Particularly it didn't worked for PR #5546:

@kwvg kwvg force-pushed the utilbps branch from 82d8a61 to f1bc1c3

Basically, one of the step gfb script -- git merge-base --fork-point develop 82d8a61d227fb66e2c6d4402ebb88e5740271eb9 returned me wrong commit. After pulling origin/develop it returns just an empty string.

But git show-branch --merge-base develop 82d8a61d227fb66e2c6d4402ebb88e5740271eb9 worked and returned correct commit:
"3e1c6dd731 fix: reorder initializations (#5545)" which is the last common commit with develop branch

What was done?

Updated recommended function function gfd() in doc

How Has This Been Tested?

Tested on PR #5546 that failed before and returned wrong diff for me

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

Seems as old command `git merge-base --fork-point` doesn't work for some cases of tree structure
Particularly it didn't worked for PR dashpay#5546:
>  @kwvg kwvg force-pushed the utilbps branch from 82d8a61 to f1bc1c3

Firstly, `git merge-base --fork-point develop 82d8a61` returned me wrong commit.
After pulling origin/develop it returns just an empty string.

But `git show-branch --merge-base develop 82d8a61` worked and returned correct commit:
  "3e1c6dd731 fix: reorder initializations (dashpay#5545)" which is the last commit for develop
@knst knst added this to the 20 milestone Sep 1, 2023
@knst knst requested a review from UdjinM6 September 1, 2023 07:57
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no thoughts; utACK for squash merge

Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@knst knst marked this pull request as draft September 6, 2023 11:12
@knst knst marked this pull request as ready for review September 6, 2023 11:15
@UdjinM6 UdjinM6 changed the title fix: improve gfb script to make it works for more cases. fix: improve gfd script to make it works for more cases. Sep 9, 2023
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work fine (tested on 5337), ACK

(fixed script name in the title)

@PastaPastaPasta PastaPastaPasta merged commit fa64c64 into dashpay:develop Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants