Skip to content

Commit a6e705a

Browse files
authored
Merge branch 'main' into feat/receipt-node-failover#1171
2 parents 6aa1960 + 99bb7cd commit a6e705a

File tree

5 files changed

+165
-22
lines changed

5 files changed

+165
-22
lines changed

.github/scripts/helpers/checks.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,37 @@ function hasVerifiedGPGSignature(commit) {
2929
}
3030

3131
/**
32-
* Checks all commits for DCO sign-off.
33-
* @param {Array<{ sha?: string, commit?: { message?: string } }>} commits
32+
* Returns true if the commit is a merge commit (has more than one parent).
33+
* Merge commits are auto-generated by Git and should be exempt from DCO sign-off.
34+
* @param {{ parents?: Array }} commit
35+
* @returns {boolean}
36+
*/
37+
function isMergeCommit(commit) {
38+
return Array.isArray(commit?.parents) && commit.parents.length > 1;
39+
}
40+
41+
/**
42+
* Checks all commits for DCO sign-off. Merge commits are skipped.
43+
* @param {Array<{ sha?: string, parents?: Array, commit?: { message?: string } }>} commits
3444
* @returns {{ passed: boolean, failures: Array<{ sha: string, message: string }> }}
3545
*/
3646
function checkDCO(commits) {
3747
const failures = [];
48+
let skipped = 0;
3849
for (const c of commits) {
50+
if (isMergeCommit(c)) {
51+
skipped++;
52+
continue;
53+
}
3954
const message = c.commit?.message || '';
4055
const shortSha = (c.sha || '').slice(0, 7);
4156
const firstLine = message.split('\n')[0] || '(no message)';
4257
if (!hasDCOSignoff(message)) {
4358
failures.push({ sha: shortSha, message: firstLine });
4459
}
4560
}
46-
getLogger().log(`DCO check: ${commits.length - failures.length}/${commits.length} passed`);
61+
const checked = commits.length - skipped;
62+
getLogger().log(`DCO check: ${checked - failures.length}/${checked} passed (${skipped} merge commit(s) skipped)`);
4763
return { passed: failures.length === 0, failures };
4864
}
4965

@@ -201,6 +217,7 @@ async function checkIssueLink(botContext, { fetchIssue, fetchClosingIssueNumbers
201217
module.exports = {
202218
hasDCOSignoff,
203219
hasVerifiedGPGSignature,
220+
isMergeCommit,
204221
checkDCO,
205222
checkGPG,
206223
checkMergeConflict,

.github/scripts/helpers/comments.js

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,15 @@ function buildSection({ title, result, passMessage }) {
4949
* @returns {string}
5050
*/
5151
function buildDCOSection(dco) {
52-
const common = buildSection({ title: 'DCO Sign-off', result: dco, passMessage: 'All commits have valid sign-offs.' });
52+
const common = buildSection({ title: 'DCO Sign-off', result: dco, passMessage: 'All commits have valid sign-offs. Nice work!' });
5353
if (common) return common;
5454

5555
const failList = (dco.failures || []).map(f => `- \`${f.sha}\` ${f.message}`).join('\n');
5656
return [
57-
':x: **DCO Sign-off** -- The following commits are missing the required DCO sign-off:',
57+
':x: **DCO Sign-off** -- Uh oh! The following commits are missing the required DCO sign-off:',
5858
failList,
5959
'',
60-
`Please add \`Signed-off-by: Your Name <email>\` to each commit (e.g. \`git commit -s\`). See the [Signing Guide](${SIGNING_GUIDE}).`,
60+
`No worries, this is an easy fix! Add \`Signed-off-by: Your Name <email>\` to each commit (e.g. \`git commit -s\`). See the [Signing Guide](${SIGNING_GUIDE}).`,
6161
].join('\n');
6262
}
6363

@@ -66,15 +66,15 @@ function buildDCOSection(dco) {
6666
* @returns {string}
6767
*/
6868
function buildGPGSection(gpg) {
69-
const common = buildSection({ title: 'GPG Signature', result: gpg, passMessage: 'All commits have verified GPG signatures.' });
69+
const common = buildSection({ title: 'GPG Signature', result: gpg, passMessage: 'All commits have verified GPG signatures. Locked and loaded!' });
7070
if (common) return common;
7171

7272
const failList = (gpg.failures || []).map(f => `- \`${f.sha}\` ${f.message}`).join('\n');
7373
return [
74-
':x: **GPG Signature** -- The following commits don\'t have a verified GPG signature:',
74+
':x: **GPG Signature** -- Heads up! The following commits don\'t have a verified GPG signature:',
7575
failList,
7676
'',
77-
`Please sign your commits with GPG (e.g. \`git commit -S\`). See the [Signing Guide](${SIGNING_GUIDE}).`,
77+
`You\'ll need to sign your commits with GPG (e.g. \`git commit -S\`). See the [Signing Guide](${SIGNING_GUIDE}) for a step-by-step walkthrough.`,
7878
].join('\n');
7979
}
8080

@@ -83,13 +83,13 @@ function buildGPGSection(gpg) {
8383
* @returns {string}
8484
*/
8585
function buildMergeSection(merge) {
86-
const common = buildSection({ title: 'Merge Conflicts', result: merge, passMessage: 'No merge conflicts detected.' });
86+
const common = buildSection({ title: 'Merge Conflicts', result: merge, passMessage: 'No merge conflicts detected. Smooth sailing!' });
8787
if (common) return common;
8888

8989
return [
90-
':x: **Merge Conflicts** -- This PR has merge conflicts with the base branch.',
90+
':x: **Merge Conflicts** -- Oh no, this PR has merge conflicts with the base branch.',
9191
'',
92-
`Please update your branch (e.g. rebase or merge from base) and push. See the [Merge Conflicts Guide](${MERGE_CONFLICTS_GUIDE}).`,
92+
`Let\'s get this sorted! Update your branch (e.g. rebase or merge from base) and push. See the [Merge Conflicts Guide](${MERGE_CONFLICTS_GUIDE}) if you need a hand.`,
9393
].join('\n');
9494
}
9595

@@ -108,15 +108,15 @@ function buildIssueLinkSection(issueLink) {
108108
if (issueLink.reason === 'not_assigned') {
109109
const unassigned = (issueLink.issues || []).filter(i => !i.isAssigned).map(i => `#${i.number}`).join(', ');
110110
return [
111-
`:x: **Issue Link** -- You are not assigned to the following linked issues: ${unassigned}.`,
111+
`:x: **Issue Link** -- Almost there! You are not assigned to the following linked issues: ${unassigned}.`,
112112
'',
113-
'Please ensure you are assigned to all linked issues before opening a PR.',
113+
'Please ensure you are assigned to all linked issues before opening a PR. You can comment `/assign` on the issue to grab it!',
114114
].join('\n');
115115
}
116116
return [
117117
':x: **Issue Link** -- This PR is not linked to any issue.',
118118
'',
119-
'Please reference an issue using a closing keyword (e.g. `Fixes #123`) and ensure the issue is assigned to you.',
119+
'Please reference an issue using a closing keyword (e.g. `Fixes #123`) and ensure the issue is assigned to you. Every PR needs a home!',
120120
].join('\n');
121121
}
122122

@@ -166,20 +166,19 @@ function allChecksPassed({ dco, gpg, merge, issueLink }) {
166166
*/
167167
function buildBotComment({ prAuthor, dco, gpg, merge, issueLink }) {
168168
const greeting = [
169-
`Hi @${prAuthor} :wave:, thank you for submitting this PR!`,
170-
"I'm your **PR Helper Bot** and I'll be keeping track of your PR's",
171-
'status to help you get it approved and merged.',
169+
`Hey @${prAuthor} :wave: thanks for the PR!`,
170+
"I'm your friendly **PR Helper Bot** :robot: and I'll be riding shotgun on this one, keeping track of your PR's status to help you get it approved and merged.",
172171
'',
173-
'This comment will stay updated as you make changes.',
174-
"Here's where things stand:",
172+
"This comment updates automatically as you push changes -- think of it as your PR's live scoreboard!",
173+
"Here's the latest:",
175174
].join('\n');
176175

177176
const checksSection = buildChecksSection({ dco, gpg, merge, issueLink });
178177
const passed = allChecksPassed({ dco, gpg, merge, issueLink });
179178

180179
const footer = passed
181-
? ':tada: *All checks passed! Your PR is ready for review.*'
182-
: '*All checks must pass before this PR can be reviewed.*';
180+
? ':tada: *All checks passed! Your PR is ready for review. Great job!*'
181+
: ':hourglass_flowing_sand: *All checks must pass before this PR can be reviewed. You\'ve got this!*';
183182

184183
const body = [MARKER, greeting, '', '---', '', checksSection, '', '---', '', footer].join('\n');
185184
return { marker: MARKER, body, allPassed: passed };

.github/scripts/tests/test-checks.js

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const { runTestSuite } = require('./test-utils');
99
const {
1010
hasDCOSignoff,
1111
hasVerifiedGPGSignature,
12+
isMergeCommit,
1213
checkDCO,
1314
checkGPG,
1415
checkMergeConflict,
@@ -85,6 +86,34 @@ const unitTests = [
8586
test: () => hasVerifiedGPGSignature(null) === false,
8687
},
8788

89+
// ---------------------------------------------------------------------------
90+
// isMergeCommit
91+
// ---------------------------------------------------------------------------
92+
{
93+
name: 'isMergeCommit: commit with two parents returns true',
94+
test: () => isMergeCommit({ parents: [{}, {}] }) === true,
95+
},
96+
{
97+
name: 'isMergeCommit: commit with one parent returns false',
98+
test: () => isMergeCommit({ parents: [{}] }) === false,
99+
},
100+
{
101+
name: 'isMergeCommit: commit with no parents field returns false',
102+
test: () => isMergeCommit({}) === false,
103+
},
104+
{
105+
name: 'isMergeCommit: null commit returns false',
106+
test: () => isMergeCommit(null) === false,
107+
},
108+
{
109+
name: 'isMergeCommit: undefined commit returns false',
110+
test: () => isMergeCommit(undefined) === false,
111+
},
112+
{
113+
name: 'isMergeCommit: commit with empty parents array returns false',
114+
test: () => isMergeCommit({ parents: [] }) === false,
115+
},
116+
88117
// ---------------------------------------------------------------------------
89118
// checkDCO
90119
// ---------------------------------------------------------------------------
@@ -133,6 +162,43 @@ const unitTests = [
133162
return r.passed === true && r.failures.length === 0;
134163
},
135164
},
165+
{
166+
name: 'checkDCO: merge commit without sign-off is skipped (passes)',
167+
test: () => {
168+
const commits = [
169+
{ sha: 'merge123', parents: [{}, {}], commit: { message: 'Merge branch main into feat' } },
170+
];
171+
const r = checkDCO(commits);
172+
return r.passed === true && r.failures.length === 0;
173+
},
174+
},
175+
{
176+
name: 'checkDCO: merge commit skipped, regular commits still checked',
177+
test: () => {
178+
const commits = [
179+
{ sha: 'abc1234', commit: { message: 'Fix\n\nSigned-off-by: A <a@x.com>' } },
180+
{ sha: 'merge56', parents: [{}, {}], commit: { message: 'Merge branch main into feat' } },
181+
{ sha: 'def5678', commit: { message: 'No sign-off here' } },
182+
];
183+
const r = checkDCO(commits);
184+
return (
185+
r.passed === false &&
186+
r.failures.length === 1 &&
187+
r.failures[0].sha === 'def5678'
188+
);
189+
},
190+
},
191+
{
192+
name: 'checkDCO: all regular commits pass with merge commit present',
193+
test: () => {
194+
const commits = [
195+
{ sha: 'abc1234', commit: { message: 'Fix\n\nSigned-off-by: A <a@x.com>' } },
196+
{ sha: 'merge56', parents: [{}, {}], commit: { message: 'Merge branch main into feat' } },
197+
];
198+
const r = checkDCO(commits);
199+
return r.passed === true && r.failures.length === 0;
200+
},
201+
},
136202

137203
// ---------------------------------------------------------------------------
138204
// checkGPG

.github/scripts/tests/test-on-pr-update-bot.js

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const {
1010
commitDCOAndGPG,
1111
commitDCOFail,
1212
commitGPGFail,
13+
commitMerge,
1314
createMockGithub,
1415
} = require('./test-utils');
1516
const script = require('../bot-on-pr-update.js');
@@ -389,6 +390,33 @@ const syncScenarios = [
389390
commentIncludes: ['All checks passed'],
390391
expectCreate: true,
391392
},
393+
{
394+
name: '[sync] Merge commit without DCO sign-off is skipped',
395+
setup: () => ({
396+
commits: [
397+
...passingCommits(),
398+
{
399+
sha: 'merge1234567890',
400+
parents: [{}, {}],
401+
commit: {
402+
message: 'Merge branch \'main\' into feat/my-feature',
403+
verification: { verified: true },
404+
},
405+
},
406+
],
407+
mergeable: true,
408+
comments: [],
409+
prLabels: [{ name: LABELS.NEEDS_REVISION }],
410+
prUser: { login: 'larry', type: 'User' },
411+
prBody: 'Fixes #100',
412+
closingIssues: [100],
413+
issues: { 100: issueWithAssignee(100, 'Fix', 'larry') },
414+
}),
415+
verify: ({ calls }) =>
416+
calls.labelsRemoved.includes(LABELS.NEEDS_REVISION) &&
417+
calls.labelsAdded.some((arr) => arr.includes(LABELS.NEEDS_REVIEW)),
418+
commentIncludes: ['All checks passed', ':white_check_mark:'],
419+
},
392420
];
393421

394422
async function runSyncTest(scenario, index) {
@@ -775,6 +803,27 @@ const editScenarios = [
775803
commentIncludes: [':x: **Issue Link**', 'not linked to any issue'],
776804
},
777805
},
806+
{
807+
name: '[edit] Merge commit without DCO sign-off is skipped',
808+
description: 'Mix of passing commit and merge commit (no sign-off). DCO still passes.',
809+
context: defaultEditContext(),
810+
githubOptions: {
811+
commits: [
812+
commitDCOAndGPG('abc1234', 'Add feature'),
813+
commitMerge('merge567', 'Merge branch \'main\' into feat/my-feature'),
814+
],
815+
mergeable: true,
816+
issues: { 42: { title: 'Bug', assignees: [{ login: 'contributor' }] } },
817+
graphqlClosingIssues: [],
818+
},
819+
expect: {
820+
labelsAdded: [],
821+
labelsRemoved: [],
822+
assignees: [],
823+
commentCreated: true,
824+
commentIncludes: [':white_check_mark:', 'All checks passed'],
825+
},
826+
},
778827
{
779828
name: '[edit] Empty changes - early exit',
780829
description: 'changes: {} or no body. Early exit.',

.github/scripts/tests/test-utils.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,17 @@ function commitGPGFail(sha, message) {
156156
};
157157
}
158158

159+
function commitMerge(sha, message) {
160+
return {
161+
sha,
162+
parents: [{}, {}],
163+
commit: {
164+
message,
165+
verification: { verified: true },
166+
},
167+
};
168+
}
169+
159170
// =============================================================================
160171
// MOCK GITHUB FACTORY
161172
// =============================================================================
@@ -264,5 +275,6 @@ module.exports = {
264275
commitDCOAndGPG,
265276
commitDCOFail,
266277
commitGPGFail,
278+
commitMerge,
267279
createMockGithub,
268280
};

0 commit comments

Comments
 (0)