Skip to content

Commit 384cdb5

Browse files
tasdomasDavidGOrtegacasperdcl
authored
Feature comment target (#1228)
* Allow creation of issue comments in cml (#1202) * Driver support for issue comments. * Create or update issue comments. * Add e2e issue comment creation tests for github and bitbucket drivers. * Add gitlab issue comment e2e test. * Add comment target flag and parsing (#1241) * Add target cli option. * Add comment target parsing. * Add debug logging in comment target parsing. Co-authored-by: DavidGOrtega <[email protected]> Co-authored-by: Casper da Costa-Luis <[email protected]>
1 parent 03e009f commit 384cdb5

13 files changed

+495
-78
lines changed

‎.github/workflows/test-deploy.yml

+3
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,15 @@ jobs:
5858
TEST_GITHUB_TOKEN: ${{ secrets.TEST_GITHUB_TOKEN }}
5959
TEST_GITHUB_REPO: https://github.com/iterative/cml_qa_tests_dummy
6060
TEST_GITHUB_SHA: 0cd16da26e35f8e5d57b2549a97e22618abf08f6
61+
TEST_GITHUB_ISSUE: 1
6162
TEST_GITLAB_TOKEN: ${{ secrets.TEST_GITLAB_TOKEN }}
6263
TEST_GITLAB_REPO: https://gitlab.com/iterative.ai/cml_qa_tests_dummy
6364
TEST_GITLAB_SHA: f8b8b49a253243830ef59a7f090eb887157b2b67
65+
TEST_GITLAB_ISSUE: 1
6466
TEST_BBCLOUD_TOKEN: ${{ secrets.TEST_BBCLOUD_TOKEN }}
6567
TEST_BBCLOUD_REPO: https://bitbucket.org/iterative-ai/cml-qa-tests-dummy
6668
TEST_BBCLOUD_SHA: b511535a89f76d3d311b1c15e3e712b15c0b94e3
69+
TEST_BBCLOUD_ISSUE: 1
6770
test-os:
6871
needs: authorize
6972
name: test-${{ matrix.system }}

‎README.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,7 @@ These are some example projects using CML.
637637
- [CML with Tensorboard](https://github.com/iterative-test/cml-example-tensorboard)
638638
- [CML with a small EC2 instance](https://github.com/iterative-test/cml-example-cloud)
639639
:key:
640-
- [CML with EC2 GPU](https://github.com/iterative-test/cml-example-cloud-gpu) :key:
640+
- [CML with EC2 GPU](https://github.com/iterative-test/cml-example-cloud-gpu)
641+
:key:
641642

642643
:key: needs a [PAT](#environment-variables).

‎bin/cml/comment/create.js

+12-3
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,25 @@ exports.builder = (yargs) =>
1818
.options(exports.options);
1919

2020
exports.options = kebabcaseKeys({
21+
target: {
22+
type: 'string',
23+
description:
24+
'Comment type (`commit`, `pr`, `commit/f00bar`, `pr/42`, `issue/1337`),' +
25+
'default is automatic (`pr` but fallback to `commit`).'
26+
},
2127
pr: {
2228
type: 'boolean',
2329
description:
24-
'Post to an existing PR/MR associated with the specified commit'
30+
'Post to an existing PR/MR associated with the specified commit',
31+
conflicts: ['target', 'commitSha'],
32+
hidden: true
2533
},
2634
commitSha: {
2735
type: 'string',
2836
alias: 'head-sha',
29-
default: 'HEAD',
30-
description: 'Commit SHA linked to this comment'
37+
description: 'Commit SHA linked to this comment',
38+
conflicts: ['target', 'pr'],
39+
hidden: true
3140
},
3241
watch: {
3342
type: 'boolean',

‎bin/cml/comment/create.test.js

+3-4
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,9 @@ describe('Comment integration tests', () => {
1717
--help Show help [boolean]
1818
1919
Options:
20-
--pr Post to an existing PR/MR associated with the
21-
specified commit [boolean]
22-
--commit-sha, --head-sha Commit SHA linked to this comment
23-
[string] [default: \\"HEAD\\"]
20+
--target Comment type (\`commit\`, \`pr\`, \`commit/f00bar\`,
21+
\`pr/42\`, \`issue/1337\`),default is automatic (\`pr\`
22+
but fallback to \`commit\`). [string]
2423
--watch Watch for changes and automatically update the
2524
comment [boolean]
2625
--publish Upload any local images found in the Markdown

‎src/cml.js

+14-48
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const winston = require('winston');
1010
const remark = require('remark');
1111
const visit = require('unist-util-visit');
1212

13+
const { parseCommentTarget } = require('./commenttarget');
1314
const Gitlab = require('./drivers/gitlab');
1415
const Github = require('./drivers/github');
1516
const BitbucketCloud = require('./drivers/bitbucket_cloud');
@@ -162,15 +163,15 @@ class CML {
162163
}
163164

164165
async commentCreate(opts = {}) {
165-
const triggerSha = await this.triggerSha();
166166
const {
167-
commitSha: inCommitSha = triggerSha,
167+
commitSha,
168168
markdownFile,
169169
pr,
170170
publish,
171171
publishUrl,
172172
report: testReport,
173173
rmWatermark,
174+
target: commentTarget = 'auto',
174175
triggerFile,
175176
update,
176177
watch,
@@ -179,9 +180,6 @@ class CML {
179180

180181
const drv = this.getDriver();
181182

182-
const commitSha =
183-
(await this.revParse({ ref: inCommitSha })) || inCommitSha;
184-
185183
if (rmWatermark && update)
186184
throw new Error('watermarks are mandatory for updateable comments');
187185

@@ -300,58 +298,26 @@ class CML {
300298
});
301299
};
302300

303-
const isBB = this.driver === BB;
304-
if (pr || isBB) {
305-
let commentUrl;
306-
307-
if (commitSha !== triggerSha)
308-
winston.info(
309-
`Looking for PR associated with --commit-sha="${inCommitSha}".\nSee https://cml.dev/doc/ref/send-comment.`
310-
);
311-
312-
const longReport = `${commitSha.substr(0, 7)}\n\n${report}`;
313-
const [commitPr = {}] = await drv.commitPrs({ commitSha });
314-
const { url } = commitPr;
315-
316-
if (!url && !isBB)
317-
throw new Error(`PR for commit sha "${inCommitSha}" not found`);
318-
319-
if (url) {
320-
const [prNumber] = url.split('/').slice(-1);
321-
322-
if (update)
323-
comment = updatableComment(await drv.prComments({ prNumber }));
324-
325-
if (update && comment) {
326-
commentUrl = await drv.prCommentUpdate({
327-
report: longReport,
328-
id: comment.id,
329-
prNumber
330-
});
331-
} else
332-
commentUrl = await drv.prCommentCreate({
333-
report: longReport,
334-
prNumber
335-
});
336-
337-
if (this.driver !== 'bitbucket') return commentUrl;
338-
}
339-
}
301+
const target = await parseCommentTarget({
302+
commitSha,
303+
pr,
304+
target: commentTarget,
305+
drv
306+
});
340307

341308
if (update) {
342-
comment = updatableComment(await drv.commitComments({ commitSha }));
309+
comment = updatableComment(await drv[target.target + 'Comments'](target));
343310

344311
if (comment)
345-
return await drv.commentUpdate({
312+
return await drv[target.target + 'CommentUpdate']({
346313
report,
347314
id: comment.id,
348-
commitSha
315+
...target
349316
});
350317
}
351-
352-
return await drv.commentCreate({
318+
return await drv[target.target + 'CommentCreate']({
353319
report,
354-
commitSha
320+
...target
355321
});
356322
}
357323

‎src/commenttarget.js

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
const winston = require('winston');
2+
3+
const SEPARATOR = '/';
4+
5+
async function parseCommentTarget(opts = {}) {
6+
const { commitSha: commit, pr, target, drv } = opts;
7+
8+
let commentTarget = target;
9+
// Handle legacy comment target flags.
10+
if (commit) {
11+
drv.warn(
12+
`Deprecation warning: use --target="commit${SEPARATOR}<sha>" instead of --commit-sha=<sha>`
13+
);
14+
commentTarget = `commit${SEPARATOR}${commit}`;
15+
}
16+
if (pr) {
17+
drv.warn('Deprecation warning: use --target=pr instead of --pr');
18+
commentTarget = 'pr';
19+
}
20+
// Handle comment targets that are incomplete, e.g. 'pr' or 'commit'.
21+
let prNumber;
22+
let commitPr;
23+
switch (commentTarget.toLowerCase()) {
24+
case 'commit':
25+
winston.debug(`Comment target "commit" mapped to "commit/${drv.sha}"`);
26+
return { target: 'commit', commitSha: drv.sha };
27+
case 'pr':
28+
case 'auto':
29+
// Determine PR id from forge env vars (if we're in a PR context).
30+
prNumber = drv.pr;
31+
if (prNumber) {
32+
winston.debug(
33+
`Comment target "${commentTarget}" mapped to "pr/${prNumber}"`
34+
);
35+
return { target: 'pr', prNumber: prNumber };
36+
}
37+
// Or fallback to determining PR by HEAD commit.
38+
// TODO: handle issue with PR HEAD commit not matching source branch in github.
39+
[commitPr = {}] = await drv.commitPrs({ commitSha: drv.sha });
40+
if (commitPr.url) {
41+
[prNumber] = commitPr.url.split('/').slice(-1);
42+
winston.debug(
43+
`Comment target "${commentTarget}" mapped to "pr/${prNumber}" based on commit "${drv.sha}"`
44+
);
45+
return { target: 'pr', prNumber };
46+
}
47+
// If target is 'auto', fallback to issuing commit comments.
48+
if (commentTarget === 'auto') {
49+
winston.debug(
50+
`Comment target "${commentTarget}" mapped to "commit/${drv.sha}"`
51+
);
52+
return { target: 'commit', commitSha: drv.sha };
53+
}
54+
throw new Error(`PR for commit sha "${drv.sha}" not found`);
55+
}
56+
// Handle qualified comment targets, e.g. 'issue/id'.
57+
const separatorPos = commentTarget.indexOf(SEPARATOR);
58+
if (separatorPos === -1) {
59+
throw new Error(`Failed to parse comment --target="${commentTarget}"`);
60+
}
61+
const targetType = commentTarget.slice(0, separatorPos);
62+
const id = commentTarget.slice(separatorPos + 1);
63+
switch (targetType.toLowerCase()) {
64+
case 'commit':
65+
return { target: targetType, commitSha: id };
66+
case 'pr':
67+
return { target: targetType, prNumber: id };
68+
case 'issue':
69+
return { target: targetType, issueId: id };
70+
default:
71+
throw new Error(`unsupported comment target "${commentTarget}"`);
72+
}
73+
}
74+
75+
module.exports = { parseCommentTarget };

0 commit comments

Comments
 (0)