Skip to content

Commit

Permalink
Rename determineMergeBaseCommitOid()
Browse files Browse the repository at this point in the history
The name suggests that the function computes the merge base, which for
Git means specifically the best common ancestors between multiple
commits or branches (see `git merge-base`).

But what the function actually does is to calculate the HEAD commit of
the PR base branch, as derived from the PR merge commit that the action
analyzes. So even though the function has to do with "merge" and "base",
using the term "merge base" is still misleading at best.

This commit renames the function to determineBaseBranchHeadCommitOid(),
which more clearly indicates what the function does.
  • Loading branch information
cklin committed Oct 3, 2024
1 parent 955d001 commit d64cca4
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 25 deletions.
12 changes: 7 additions & 5 deletions lib/actions-util.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/actions-util.js.map

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions lib/actions-util.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/actions-util.test.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion lib/upload-lib.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/upload-lib.js.map

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions src/actions-util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,26 +269,26 @@ test("isAnalyzingDefaultBranch()", async (t) => {
});
});

test("determineMergeBaseCommitOid non-pullrequest", async (t) => {
test("determineBaseBranchHeadCommitOid non-pullrequest", async (t) => {
const infoStub = sinon.stub(core, "info");

process.env["GITHUB_EVENT_NAME"] = "hucairz";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
const result = await actionsUtil.determineMergeBaseCommitOid(__dirname);
const result = await actionsUtil.determineBaseBranchHeadCommitOid(__dirname);
t.deepEqual(result, undefined);
t.deepEqual(0, infoStub.callCount);

infoStub.restore();
});

test("determineMergeBaseCommitOid not git repository", async (t) => {
test("determineBaseBranchHeadCommitOid not git repository", async (t) => {
const infoStub = sinon.stub(core, "info");

process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";

await withTmpDir(async (tmpDir) => {
await actionsUtil.determineMergeBaseCommitOid(tmpDir);
await actionsUtil.determineBaseBranchHeadCommitOid(tmpDir);
});

t.deepEqual(1, infoStub.callCount);
Expand All @@ -301,12 +301,12 @@ test("determineMergeBaseCommitOid not git repository", async (t) => {
infoStub.restore();
});

test("determineMergeBaseCommitOid other error", async (t) => {
test("determineBaseBranchHeadCommitOid other error", async (t) => {
const infoStub = sinon.stub(core, "info");

process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
const result = await actionsUtil.determineMergeBaseCommitOid(
const result = await actionsUtil.determineBaseBranchHeadCommitOid(
path.join(__dirname, "../../i-dont-exist"),
);
t.deepEqual(result, undefined);
Expand Down
8 changes: 5 additions & 3 deletions src/actions-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,12 @@ export const getCommitOid = async function (
};

/**
* If the action was triggered by a pull request, determine the commit sha of the merge base.
* Returns undefined if run by other triggers or the merge base cannot be determined.
* If the action was triggered by a pull request, determine the commit sha at
* the head of the base branch, using the merge commit that this workflow analyzes.
* Returns undefined if run by other triggers or the base branch commit cannot be
* determined.
*/
export const determineMergeBaseCommitOid = async function (
export const determineBaseBranchHeadCommitOid = async function (
checkoutPathOverride?: string,
): Promise<string | undefined> {
if (getWorkflowEventName() !== "pull_request") {
Expand Down
2 changes: 1 addition & 1 deletion src/upload-lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ export async function uploadFiles(
checkoutURI,
environment,
toolNames,
await actionsUtil.determineMergeBaseCommitOid(),
await actionsUtil.determineBaseBranchHeadCommitOid(),
);

// Log some useful debug info about the info
Expand Down

0 comments on commit d64cca4

Please sign in to comment.