Skip to content

fix: improve gfd script to make it works for more cases.#5561

Merged
PastaPastaPasta merged 1 commit into
dashpay:developfrom
knst:fix-contributing-gfb
Sep 9, 2023
Merged

fix: improve gfd script to make it works for more cases.#5561
PastaPastaPasta merged 1 commit into
dashpay:developfrom
knst:fix-contributing-gfb

Conversation

@knst

@knst knst commented Sep 1, 2023

Copy link
Copy Markdown
Collaborator

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 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 (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

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member

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

@kwvg kwvg left a comment

Copy link
Copy Markdown
Collaborator

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

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

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