Skip to content
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

Add comment target flag and parsing #1241

Merged
merged 14 commits into from
Nov 9, 2022
14 changes: 8 additions & 6 deletions bin/cml/comment/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,23 @@ exports.builder = (yargs) =>
.options(exports.options);

exports.options = kebabcaseKeys({
target: {
type: 'string',
description:
'Forge object to create comment on, can be one of pr, commit or issue. ' +
"Specify 'issue#123' to create a comment on a specific issue. " +
Copy link
Contributor

@casperdcl casperdcl Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what syntaxes are supported?

  • 123
  • #123
  • issue#123
  • pr#123
  • mr#123
  • URL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid targets are:

  • auto determine comment target automatically:
    • PR if running in a PR context
    • PR if HEAD commit is part of a PR
    • HEAD commit (fallback)
  • pr - determine pr by context or head commit (making allowances for how github's PR merge commits)
  • pr#123 - specific PR number
  • commit - current HEAD commit
  • commit#adfa12 - specific commit
  • issue#123 - specific issue

Copy link
Contributor

@casperdcl casperdcl Nov 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need:

  • #123 to auto-infer pr-or-issue
  • mr alias for pr
  • mr#123 alias for pr#123
  • why do we have commit#d34db3? Doesn't --commit-sha=d34db3 already cover this?
  • full URL (e.g. if they want to target another PR returned by cml pr)

moved to #1228 (review)

'By default cml will create a PR comment if running in a forge PR-related action ' +
'or if HEAD is in a PR branch. Otherwise a commit comment will be created.'
},
pr: {
type: 'boolean',
description:
'Post to an existing PR/MR associated with the specified commit',
conflicts: ['issue']
tasdomas marked this conversation as resolved.
Show resolved Hide resolved
},
issue: {
type: 'number',
description: 'Post to the given issue number',
conflicts: ['pr']
},
commitSha: {
type: 'string',
alias: 'head-sha',
default: 'HEAD',
description: 'Commit SHA linked to this comment'
},
watch: {
Expand Down
11 changes: 8 additions & 3 deletions bin/cml/comment/create.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,16 @@ describe('Comment integration tests', () => {
--help Show help [boolean]

Options:
--target Forge object to create comment on, can be one of
pr, commit or issue. Specify 'issue#123' to create
a comment on a specific issue. The default 'auto'
will create a PR comment if running in a forge
PR-related action or if HEAD is in a PR branch.
Otherwise a commit comment will be created.
[string] [default: \\"auto\\"]
--pr Post to an existing PR/MR associated with the
specified commit [boolean]
--issue Post to the given issue number [number]
--commit-sha, --head-sha Commit SHA linked to this comment
[string] [default: \\"HEAD\\"]
--commit-sha, --head-sha Commit SHA linked to this comment [string]
--watch Watch for changes and automatically update the
comment [boolean]
--publish Upload any local images found in the Markdown
Expand Down
80 changes: 14 additions & 66 deletions src/cml.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const winston = require('winston');
const remark = require('remark');
const visit = require('unist-util-visit');

const { parseCommentTarget } = require('./commenttarget');
const Gitlab = require('./drivers/gitlab');
const Github = require('./drivers/github');
const BitbucketCloud = require('./drivers/bitbucket_cloud');
Expand Down Expand Up @@ -162,16 +163,15 @@ class CML {
}

async commentCreate(opts = {}) {
const triggerSha = await this.triggerSha();
DavidGOrtega marked this conversation as resolved.
Show resolved Hide resolved
const {
commitSha: inCommitSha = triggerSha,
issue: issueId,
commitSha: inCommitSha,
tasdomas marked this conversation as resolved.
Show resolved Hide resolved
markdownFile,
pr,
publish,
publishUrl,
report: testReport,
rmWatermark,
target: commentTarget = 'auto',
triggerFile,
update,
watch,
Expand All @@ -180,9 +180,6 @@ class CML {

const drv = this.getDriver();

const commitSha =
(await this.revParse({ ref: inCommitSha })) || inCommitSha;

if (rmWatermark && update)
throw new Error('watermarks are mandatory for updateable comments');

Expand Down Expand Up @@ -300,76 +297,27 @@ class CML {
return body.includes(watermark);
});
};
// Create or update an issue comment.
if (issueId) {
if (update) {
comment = updatableComment(await drv.issueComments({ issueId }));

if (comment)
return await drv.issueCommentUpdate({
report,
id: comment.id,
issueId
});
}
return await drv.issueCommentCreate({
report,
issueId
});
}

const isBB = this.driver === BB;
if (pr || isBB) {
let commentUrl;

if (commitSha !== triggerSha)
winston.info(
`Looking for PR associated with --commit-sha="${inCommitSha}".\nSee https://cml.dev/doc/ref/send-comment.`
);

const longReport = `${commitSha.substr(0, 7)}\n\n${report}`;
const [commitPr = {}] = await drv.commitPrs({ commitSha });
const { url } = commitPr;

if (!url && !isBB)
throw new Error(`PR for commit sha "${inCommitSha}" not found`);

if (url) {
const [prNumber] = url.split('/').slice(-1);

if (update)
comment = updatableComment(await drv.prComments({ prNumber }));

if (update && comment) {
commentUrl = await drv.prCommentUpdate({
report: longReport,
id: comment.id,
prNumber
});
} else
commentUrl = await drv.prCommentCreate({
report: longReport,
prNumber
});

if (this.driver !== 'bitbucket') return commentUrl;
}
}
const target = await parseCommentTarget({
inCommitSha,
pr,
target: commentTarget,
drv
});

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

if (comment)
return await drv.commentUpdate({
return await drv[target.target + 'CommentUpdate']({
report,
id: comment.id,
commitSha
...target
});
}

return await drv.commentCreate({
return await drv[target.target + 'CommentCreate']({
report,
commitSha
...target
});
}

Expand Down
63 changes: 63 additions & 0 deletions src/commenttarget.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
const SEPARATOR = '#';

async function parseCommentTarget(opts = {}) {
const { commitSha: commit, pr, target, drv } = opts;

let commentTarget = target;
// Handle legacy comment target flags.
if (commit) {
drv.warn(
'cml: the --commitSha flag will be deprecated, please use --target="commit#<sha>"'
);
commentTarget = `commit#${commit}`;
}
if (pr) {
drv.warn('cml: the --pr flag will be deprecated, please use --target="pr"');
commentTarget = 'pr';
}
// Handle comment targets that are incomplete, e.g. 'pr' or 'commit'.
let prId;
let commitPr;
switch (commentTarget) {
case 'commit':
return { target: 'commit', commitSha: drv.sha };
case 'pr':
case 'auto':
// Determine PR id from forge env vars (if we're in a PR context).
prId = drv.pr;
if (prId) {
return { target: 'pr', prNumber: prId };
tasdomas marked this conversation as resolved.
Show resolved Hide resolved
}
// Or fallback to determining PR by HEAD commit.
// TODO: handle issue with PR HEAD commit not matching source branch in github.
[commitPr = {}] = await drv.commitPrs({ commitSha: drv.sha });
if (commitPr.url) {
[prId] = commitPr.url.split('/').slice(-1);
return { target: 'pr', prNumber: prId };
}
// If target is 'auto', fallback to issuing commit comments.
if (commentTarget === 'auto') {
return { target: 'commit', commitSha: drv.sha };
}
throw new Error(`PR for commit sha "${drv.sha}" not found`);
}
// Handle qualified comment targets, e.g. 'issue#id'.
const separatorPos = commentTarget.indexOf(SEPARATOR);
if (separatorPos === -1) {
throw new Error(`comment target "${commentTarget}" could not be parsed`);
}
const targetType = commentTarget.slice(0, separatorPos);
const id = commentTarget.slice(separatorPos + 1);
switch (targetType) {
case 'commit':
return { target: targetType, commitSha: id };
case 'pr':
return { target: targetType, prNumber: id };
case 'issue':
return { target: targetType, issueId: id };
default:
throw new Error(`unsupported comment target "${commentTarget}"`);
}
}

module.exports = { parseCommentTarget };
Loading