fix(IDX): update diff logic to work outside of PRs #1745
Conversation
This reverts commit e26a05e.
| @@ -13,7 +13,11 @@ set -euo pipefail | |||
| set -x | |||
| cd "$(git rev-parse --show-toplevel)" | |||
|
|
|||
| MERGE_BASE="${MERGE_BASE_SHA:-HEAD}" | |||
| # since default branch is not the same in ic-private, we can't set it to master, throw error if missing | |||
| git fetch origin $DEFAULT_BRANCH | |||
There was a problem hiding this comment.
This would cause quite some downloads since we don't fetch all history when we checkout the repo.
There was a problem hiding this comment.
But it's only pulling master... Or should I specify a couple commits like we have in the checkout step?
There was a problem hiding this comment.
Using --depth 512 should be safe enough.
| git fetch origin $DEFAULT_BRANCH | ||
| TARGET_BRANCH_NAME="${TARGET_BRANCH_NAME:-$DEFAULT_BRANCH}" | ||
| TARGET_BRANCH_SHA=$(git rev-parse origin/$TARGET_BRANCH_NAME) | ||
| MERGE_BASE=$(git merge-base HEAD "$TARGET_BRANCH_SHA") |
There was a problem hiding this comment.
Wouldn't this cause the problem we discussed yesterday where HEAD on PRs points to a merge between the PR branch and the target branch meaning that this git merge-base HEAD "$TARGET_BRANCH_SHA" will return that merge commit which then results in COMMIT_RANGE being HEAD.. which then causes DIFF_FILES to become empty?
I'm doing these git calculations in my head so I could be wrong here.
There was a problem hiding this comment.
Oh I realise I'm wrong. If HEAD points to a merge of the PR with the target branch git merge-base HEAD "$TARGET_BRANCH_SHA" should result in $TARGET_BRANCH_SHA.
Meaning COMMIT_RANGE=$TARGET_BRANCH_SHA... So that could actually work.
There was a problem hiding this comment.
Yeah I double checked this and from my understanding git merge-base will always return the common ancestor (so not the merge commit). It's the same as MERGE_BASE_SHA: ${{ github.event.pull_request.base.sha }}. The one that could return a merge commit would be the latest commit, which is CI_COMMIT_SHA: ${{ github.sha }}.
This PR contains two changes:
diff.shto compare differences against the default branch if it was not created on a PR and therefore noTARGET_BRANCHis available.CI_PULL_REQUEST_TARGET_BRANCH_NAMEtoTARGET_BRANCH_NAMEfor simplicity