-
Notifications
You must be signed in to change notification settings - Fork 142
feat: Add Annotated Tag Push Support #1139
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: main
Are you sure you want to change the base?
Conversation
# Conflicts: # src/ui/views/OpenPushRequests/components/PushesTable.tsx # src/ui/views/PushDetails/PushDetails.tsx
✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1139 +/- ##
==========================================
+ Coverage 83.25% 83.44% +0.18%
==========================================
Files 59 60 +1
Lines 2449 2561 +112
Branches 280 318 +38
==========================================
+ Hits 2039 2137 +98
- Misses 366 369 +3
- Partials 44 55 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
} else if (action.tagData?.length) { | ||
action.user = action.tagData.at(-1)!.tagger; | ||
} else { | ||
throw new Error('No commit or tag data parsed from packfile'); |
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.
It seems that the commitData
check here is causing this test to fail:
it.only('should return empty commitData on empty branch push', async () => {
const emptyPackBuffer = createEmptyPackBuffer();
const newCommit = 'b'.repeat(40);
const ref = 'refs/heads/feature/emptybranch';
const packetLine = `${EMPTY_COMMIT_HASH} ${newCommit} ${ref}\0capabilities\n`;
req.body = Buffer.concat([createPacketLineBuffer([packetLine]), emptyPackBuffer]);
const result = await exec(req, action);
expect(result).to.equal(action);
const step = action.steps.find(s => s.stepName === 'parsePackFile');
expect(step).to.exist;
expect(step.error).to.be.false; // <-- Fails here
expect(action.branch).to.equal(ref);
expect(action.setCommit.calledOnceWith(EMPTY_COMMIT_HASH, newCommit)).to.be.true;
expect(action.commitData).to.be.an('array').with.lengthOf(0);
});
Ideally, we want to allow empty commitData
so that we can interpret it later: It might be an empty branch push, a tag push or something else entirely.
Perhaps we could move this check into the another action? We could rename the checkEmptyBranch
action to handleEmptyCommitData
and do all the relevant checks there.
); | ||
} finally { | ||
action.addStep(step); | ||
} | ||
return action; | ||
} | ||
|
||
function parseTag(x: CommitContent): TagData { |
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.
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.
The core flow seems to work correctly! Just a few edge cases and an integration issue with the checkEmptyBranch
that might be worth refactoring.
Also, we will be merging #973 soon which will likely cause a few conflicts here... Might want an extra pair of eyes to make sure everything is good to merge.
Description:
This PR introduces end-to-end support for annotated tag pushes
branchPushChain
andtagPushChain
.getChain()
to select the correct chain based onaction.type
and presence ofaction.tag
.Fixes #986
Note: Restored from deleted fork
This PR recreates the original PR #1051, which was automatically closed due to accidental fork deletion.
** For discussions and reviews:** See the original PR #1051
All commits are identical to the original with preserved git history.