From e70fc384df2fe94725e2e447637ef4097d78d040 Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Tue, 18 Oct 2022 16:04:41 +0300 Subject: [PATCH 01/13] Add target cli option. --- bin/cml/comment/create.js | 7 +++++++ bin/cml/comment/create.test.js | 3 +++ 2 files changed, 10 insertions(+) diff --git a/bin/cml/comment/create.js b/bin/cml/comment/create.js index b32996433..b17a5f2dd 100644 --- a/bin/cml/comment/create.js +++ b/bin/cml/comment/create.js @@ -18,6 +18,13 @@ 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.", + conflicts: ['pr', 'issue', 'commitSha'] + }, pr: { type: 'boolean', description: diff --git a/bin/cml/comment/create.test.js b/bin/cml/comment/create.test.js index 68cd40d09..72eefa3a7 100644 --- a/bin/cml/comment/create.test.js +++ b/bin/cml/comment/create.test.js @@ -18,6 +18,9 @@ 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. [string] --pr Post to an existing PR/MR associated with the specified commit [boolean] --issue Post to the given issue number [number] From 2324313174da9ac6c24afd6161cb465b5a87a2c8 Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Fri, 28 Oct 2022 16:22:45 +0300 Subject: [PATCH 02/13] Add comment target parsing. --- src/commenttarget.js | 64 +++++++++++++++++++ src/commenttarget.test.js | 130 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 src/commenttarget.js create mode 100644 src/commenttarget.test.js diff --git a/src/commenttarget.js b/src/commenttarget.js new file mode 100644 index 000000000..52a83304a --- /dev/null +++ b/src/commenttarget.js @@ -0,0 +1,64 @@ +const SEPARATOR = '#'; + +async function parseCommentTarget(opts = {}) { + const { commitSha: commit, issue, pr, target, drv } = opts; + + let commentTarget = target; + // Handle legacy comment target flags. + if (issue) { + drv.warn( + 'cml: the --issue flag will be deprecated, please use --target="issue#123"' + ); + commentTarget = `issue#${issue}`; + } + if (commit) { + drv.warn( + 'cml: the --commitSha flag will be deprecated, please use --target="commit#"' + ); + 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': + // Determine PR id from forge env vars (if we're in a PR context). + prId = drv.pr; + if (prId) { + return { target: 'pr', prNumber: prId }; + } + // 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) { + throw new Error(`PR for commit sha "${drv.sha}" not found`); + } + [prId] = commitPr.url.split('/').slice(-1); + return { target: 'pr', prNumber: prId }; + } + // 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 }; diff --git a/src/commenttarget.test.js b/src/commenttarget.test.js new file mode 100644 index 000000000..42ee226ab --- /dev/null +++ b/src/commenttarget.test.js @@ -0,0 +1,130 @@ +const { parseCommentTarget } = require('../src/commenttarget'); + +describe('comment target tests', () => { + beforeEach(() => { + jest.resetModules(); + }); + + test('qualified comment target: pr', async () => { + const target = await parseCommentTarget({ + target: 'pr#3' + }); + expect(target).toEqual({ target: 'pr', prNumber: '3' }); + }); + + test('qualified comment target: commit', async () => { + const target = await parseCommentTarget({ + target: 'commit#abcdefg' + }); + expect(target).toEqual({ target: 'commit', commitSha: 'abcdefg' }); + }); + + test('qualified comment target: issue', async () => { + const target = await parseCommentTarget({ + target: 'issue#3' + }); + expect(target).toEqual({ target: 'issue', issueId: '3' }); + }); + + test('qualified comment target: unsupported', async () => { + try { + await parseCommentTarget({ + target: 'unsupported#3' + }); + } catch (error) { + expect(error.message).toBe('unsupported comment target "unsupported#3"'); + } + }); + + test('legacy flag: commitSha', async () => { + const drv = { warn: () => {} }; + const target = await parseCommentTarget({ + drv, + commitSha: 'abcdefg', + target: 'issue#3' // target will be replaced with commit + }); + expect(target).toEqual({ target: 'commit', commitSha: 'abcdefg' }); + }); + + // Test retrieving the PR id from the driver's context. + test('legacy flag: pr, context', async () => { + const drv = { + warn: () => {}, + pr: '4' // driver returns the PR id from context + }; + const target = await parseCommentTarget({ + drv, + pr: true, + target: 'issue#3' // target will be replaced + }); + expect(target).toEqual({ target: 'pr', prNumber: '4' }); + }); + + // Test calculating the PR id based on the commit sha. + test('legacy flag: pr, commit sha', async () => { + const drv = { + warn: () => {}, + pr: null, // not in PR context + commitPrs: () => [{ url: 'forge/pr/4' }], + sha: 'abcdefg' + }; + const target = await parseCommentTarget({ + drv, + pr: true, + target: 'issue#3' // target will be replaced + }); + expect(target).toEqual({ target: 'pr', prNumber: '4' }); + }); + + // Test using driver supplied commit sha. + test('unqualified flag: commit sha', async () => { + const drv = { + warn: () => {}, + sha: 'abcdefg' + }; + const target = await parseCommentTarget({ + drv, + target: 'commit' + }); + expect(target).toEqual({ target: 'commit', commitSha: 'abcdefg' }); + }); + + // Test retrieving the PR id from the driver's context. + test('unqualified flag: pr, context', async () => { + const drv = { + warn: () => {}, + pr: '4' // driver returns the PR id from context + }; + const target = await parseCommentTarget({ + drv, + target: 'pr' // target will be replaced + }); + expect(target).toEqual({ target: 'pr', prNumber: '4' }); + }); + + // Test calculating the PR id based on the commit sha. + test('unqualified flag: pr, commit sha', async () => { + const drv = { + warn: () => {}, + pr: null, // not in PR context + commitPrs: () => [{ url: 'forge/pr/4' }], + sha: 'abcdefg' + }; + const target = await parseCommentTarget({ + drv, + pr: true, + target: 'pr' // target will be replaced + }); + expect(target).toEqual({ target: 'pr', prNumber: '4' }); + }); + + test('unqualified comment target: issue', async () => { + try { + await parseCommentTarget({ + target: 'issue' + }); + } catch (error) { + expect(error.message).toBe('comment target "issue" could not be parsed'); + } + }); +}); From 17512f104f26b60d3e7974ac31dddc53f61d8d3f Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Fri, 28 Oct 2022 16:52:55 +0300 Subject: [PATCH 03/13] Update driver code. Add warn method. Rename commit comment methods with 'commit' prefix. --- src/drivers/bitbucket_cloud.e2e.test.js | 2 +- src/drivers/bitbucket_cloud.js | 18 ++++++++++++++++-- src/drivers/github.e2e.test.js | 2 +- src/drivers/github.js | 14 ++++++++++++-- src/drivers/gitlab.e2e.test.js | 2 +- src/drivers/gitlab.js | 18 ++++++++++++++++-- 6 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/drivers/bitbucket_cloud.e2e.test.js b/src/drivers/bitbucket_cloud.e2e.test.js index 7bc341c2a..8970e189c 100644 --- a/src/drivers/bitbucket_cloud.e2e.test.js +++ b/src/drivers/bitbucket_cloud.e2e.test.js @@ -25,7 +25,7 @@ describe('Non Enviromental tests', () => { test('Comment', async () => { const report = '## Test comment'; const commitSha = SHA; - const url = await client.commentCreate({ report, commitSha }); + const url = await client.commitCommentCreate({ report, commitSha }); expect(url.startsWith(REPO)).toBe(true); }); diff --git a/src/drivers/bitbucket_cloud.js b/src/drivers/bitbucket_cloud.js index e5d13698b..76782fd65 100644 --- a/src/drivers/bitbucket_cloud.js +++ b/src/drivers/bitbucket_cloud.js @@ -67,7 +67,7 @@ class BitbucketCloud { ); } - async commentCreate(opts = {}) { + async commitCommentCreate(opts = {}) { const { projectPath } = this; const { commitSha, report } = opts; @@ -81,7 +81,7 @@ class BitbucketCloud { ).links.html.href; } - async commentUpdate(opts = {}) { + async commitCommentUpdate(opts = {}) { const { projectPath } = this; const { commitSha, report, id } = opts; @@ -463,6 +463,16 @@ class BitbucketCloud { return BITBUCKET_COMMIT; } + /** + * Returns the PR number if we're in a PR-related action event. + */ + get pr() { + if ('BITBUCKET_PR_ID' in process.env) { + return process.env.BITBUCKET_PR_ID; + } + return null; + } + get branch() { return BITBUCKET_BRANCH; } @@ -524,6 +534,10 @@ class BitbucketCloud { return responseBody; } + + warn(message) { + console.error(message); + } } module.exports = BitbucketCloud; diff --git a/src/drivers/github.e2e.test.js b/src/drivers/github.e2e.test.js index 9d1b87be9..f39bdbdda 100644 --- a/src/drivers/github.e2e.test.js +++ b/src/drivers/github.e2e.test.js @@ -31,7 +31,7 @@ describe('Non Enviromental tests', () => { test('Comment', async () => { const report = '## Test comment'; const commitSha = SHA; - const url = await client.commentCreate({ report, commitSha }); + const url = await client.commitCommentCreate({ report, commitSha }); expect(url.startsWith(REPO)).toBe(true); }); diff --git a/src/drivers/github.js b/src/drivers/github.js index 15663292a..c0d617b1d 100644 --- a/src/drivers/github.js +++ b/src/drivers/github.js @@ -100,7 +100,7 @@ class Github { return user; } - async commentCreate(opts = {}) { + async commitCommentCreate(opts = {}) { const { report: body, commitSha } = opts; const { repos } = octokit(this.token, this.repo); @@ -113,7 +113,7 @@ class Github { ).data.html_url; } - async commentUpdate(opts = {}) { + async commitCommentUpdate(opts = {}) { const { report: body, id } = opts; const { repos } = octokit(this.token, this.repo); @@ -767,6 +767,16 @@ class Github { return GITHUB_SHA; } + /** + * Returns the PR number if we're in a PR-related action event. + */ + get pr() { + if (['pull_request', 'pull_request_target'].includes(GITHUB_EVENT_NAME)) { + return github.context.payload.pull_request.number; + } + return null; + } + get branch() { return branchName(GITHUB_HEAD_REF || GITHUB_REF); } diff --git a/src/drivers/gitlab.e2e.test.js b/src/drivers/gitlab.e2e.test.js index 44e37bacc..4462b8314 100644 --- a/src/drivers/gitlab.e2e.test.js +++ b/src/drivers/gitlab.e2e.test.js @@ -26,7 +26,7 @@ describe('Non Enviromental tests', () => { test('Comment', async () => { const report = '## Test comment'; const commitSha = SHA; - const url = await client.commentCreate({ + const url = await client.commitCommentCreate({ report, commitSha }); diff --git a/src/drivers/gitlab.js b/src/drivers/gitlab.js index 908a39384..9b52dd1f0 100644 --- a/src/drivers/gitlab.js +++ b/src/drivers/gitlab.js @@ -65,7 +65,7 @@ class Gitlab { return this.detectedBase; } - async commentCreate(opts = {}) { + async commitCommentCreate(opts = {}) { const { commitSha, report } = opts; const projectPath = await this.projectPath(); @@ -78,7 +78,7 @@ class Gitlab { return `${this.repo}/-/commit/${commitSha}`; } - async commentUpdate(opts = {}) { + async commitCommentUpdate(opts = {}) { throw new Error('GitLab does not support comment updates!'); } @@ -493,6 +493,16 @@ class Gitlab { return process.env.CI_COMMIT_SHA; } + /** + * Returns the PR number if we're in a PR-related action event. + */ + get pr() { + if ('CI_MERGE_REQUEST_IID' in process.env) { + return process.env.CI_MERGE_REQUEST_IID; + } + return null; + } + get branch() { return process.env.CI_BUILD_REF_NAME; } @@ -527,6 +537,10 @@ class Gitlab { return await response.json(); } + + warn(message) { + console.error(message); + } } module.exports = Gitlab; From 5b1deb4e8e03cbd4304a9d31398c6efbe9bfbdbe Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Fri, 28 Oct 2022 17:01:18 +0300 Subject: [PATCH 04/13] Use comment target parsing. --- src/cml.js | 79 ++++++++++++------------------------------------------ 1 file changed, 17 insertions(+), 62 deletions(-) diff --git a/src/cml.js b/src/cml.js index 1cba84086..97bdc0bd9 100755 --- a/src/cml.js +++ b/src/cml.js @@ -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'); @@ -161,9 +162,8 @@ class CML { } async commentCreate(opts = {}) { - const triggerSha = await this.triggerSha(); const { - commitSha: inCommitSha = triggerSha, + commitSha: inCommitSha, issue: issueId, rmWatermark, update, @@ -172,6 +172,7 @@ class CML { publishUrl, markdownFile, report: testReport, + target: commentTarget, watch, triggerFile } = opts; @@ -268,76 +269,30 @@ class CML { return body.includes('watermark.svg'); }); }; - // 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({ + commitSha, + issue: issueId, + pr, + target: commentTarget, + drv + }); if (update) { - comment = updatableComment(await drv.commitComments({ commitSha })); + comment = updatableComment( + await drv[target.target + 'Comments']({ issueId }) + ); 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 }); } From 6b99ce900a1a923e139e71b2cf2aa3d51b64a778 Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Thu, 3 Nov 2022 10:49:33 +0200 Subject: [PATCH 05/13] Remove --issue flag for comment create. The flag is replaced with --target=issue#. --- bin/cml/comment/create.js | 5 ----- bin/cml/comment/create.test.js | 1 - src/cml.js | 6 +----- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/bin/cml/comment/create.js b/bin/cml/comment/create.js index b17a5f2dd..42fbe6449 100644 --- a/bin/cml/comment/create.js +++ b/bin/cml/comment/create.js @@ -31,11 +31,6 @@ exports.options = kebabcaseKeys({ 'Post to an existing PR/MR associated with the specified commit', conflicts: ['issue'] }, - issue: { - type: 'number', - description: 'Post to the given issue number', - conflicts: ['pr'] - }, commitSha: { type: 'string', alias: 'head-sha', diff --git a/bin/cml/comment/create.test.js b/bin/cml/comment/create.test.js index 72eefa3a7..5ca1974b2 100644 --- a/bin/cml/comment/create.test.js +++ b/bin/cml/comment/create.test.js @@ -23,7 +23,6 @@ describe('Comment integration tests', () => { a comment on a specific issue. [string] --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\\"] --watch Watch for changes and automatically update the diff --git a/src/cml.js b/src/cml.js index 97bdc0bd9..e13ac5012 100755 --- a/src/cml.js +++ b/src/cml.js @@ -164,7 +164,6 @@ class CML { async commentCreate(opts = {}) { const { commitSha: inCommitSha, - issue: issueId, rmWatermark, update, pr, @@ -272,16 +271,13 @@ class CML { const target = await parseCommentTarget({ commitSha, - issue: issueId, pr, target: commentTarget, drv }); if (update) { - comment = updatableComment( - await drv[target.target + 'Comments']({ issueId }) - ); + comment = updatableComment(await drv[target.target + 'Comments'](target)); if (comment) return await drv[target.target + 'CommentUpdate']({ From 85281d9fe164d4df1f690259ed2e0b3ff0f05793 Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Thu, 3 Nov 2022 11:45:38 +0200 Subject: [PATCH 06/13] Handle --target=auto for comment create. --target=auto will create PR comments if the comment is run in an action with a PR context or if the HEAD commit is associated with a PR. Otherwise a commit comment will be created. --- bin/cml/comment/create.js | 9 ++++--- bin/cml/comment/create.test.js | 8 +++++-- src/commenttarget.js | 13 ++++++---- src/commenttarget.test.js | 43 ++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 9 deletions(-) diff --git a/bin/cml/comment/create.js b/bin/cml/comment/create.js index 42fbe6449..90549e00b 100644 --- a/bin/cml/comment/create.js +++ b/bin/cml/comment/create.js @@ -21,9 +21,12 @@ 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.", - conflicts: ['pr', 'issue', 'commitSha'] + '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.', + conflicts: ['pr', 'issue', 'commitSha'], + default: 'auto' }, pr: { type: 'boolean', diff --git a/bin/cml/comment/create.test.js b/bin/cml/comment/create.test.js index 5ca1974b2..b51887d48 100644 --- a/bin/cml/comment/create.test.js +++ b/bin/cml/comment/create.test.js @@ -19,8 +19,12 @@ describe('Comment integration tests', () => { 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. [string] + 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] --commit-sha, --head-sha Commit SHA linked to this comment diff --git a/src/commenttarget.js b/src/commenttarget.js index 52a83304a..227e93edc 100644 --- a/src/commenttarget.js +++ b/src/commenttarget.js @@ -28,6 +28,7 @@ async function parseCommentTarget(opts = {}) { 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) { @@ -36,11 +37,15 @@ async function parseCommentTarget(opts = {}) { // 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) { - throw new Error(`PR for commit sha "${drv.sha}" not found`); + 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 }; } - [prId] = commitPr.url.split('/').slice(-1); - return { target: 'pr', prNumber: prId }; + throw new Error(`PR for commit sha "${drv.sha}" not found`); } // Handle qualified comment targets, e.g. 'issue#id'. const separatorPos = commentTarget.indexOf(SEPARATOR); diff --git a/src/commenttarget.test.js b/src/commenttarget.test.js index 42ee226ab..5d3c3f99e 100644 --- a/src/commenttarget.test.js +++ b/src/commenttarget.test.js @@ -127,4 +127,47 @@ describe('comment target tests', () => { expect(error.message).toBe('comment target "issue" could not be parsed'); } }); + + test('auto comment target: pr context', async () => { + const drv = { + warn: () => {}, + pr: '4' // driver returns the PR id from context + }; + + const target = await parseCommentTarget({ + drv, + target: 'auto' + }); + expect(target).toEqual({ target: 'pr', prNumber: '4' }); + }); + + test('auto comment target: pr, commit sha', async () => { + const drv = { + warn: () => {}, + pr: null, // not in PR context + commitPrs: () => [{ url: 'forge/pr/5' }], + sha: 'abcdefg' + }; + + const target = await parseCommentTarget({ + drv, + target: 'auto' + }); + expect(target).toEqual({ target: 'pr', prNumber: '5' }); + }); + + test('auto comment target: fallback commit sha', async () => { + const drv = { + warn: () => {}, + pr: null, // not in PR context + commitPrs: () => [], + sha: 'abcdefg' + }; + + const target = await parseCommentTarget({ + drv, + target: 'auto' + }); + expect(target).toEqual({ target: 'commit', commitSha: 'abcdefg' }); + }); }); From f1f4f626745b0b7861e2c39a37425919568ad8d1 Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Thu, 3 Nov 2022 12:47:02 +0200 Subject: [PATCH 07/13] Remove default commitSha flag value. --- bin/cml/comment/create.js | 1 - bin/cml/comment/create.test.js | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/bin/cml/comment/create.js b/bin/cml/comment/create.js index 90549e00b..1c93af9cb 100644 --- a/bin/cml/comment/create.js +++ b/bin/cml/comment/create.js @@ -37,7 +37,6 @@ exports.options = kebabcaseKeys({ commitSha: { type: 'string', alias: 'head-sha', - default: 'HEAD', description: 'Commit SHA linked to this comment' }, watch: { diff --git a/bin/cml/comment/create.test.js b/bin/cml/comment/create.test.js index b51887d48..e72b9d673 100644 --- a/bin/cml/comment/create.test.js +++ b/bin/cml/comment/create.test.js @@ -27,8 +27,7 @@ describe('Comment integration tests', () => { [string] [default: \\"auto\\"] --pr Post to an existing PR/MR associated with the specified commit [boolean] - --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 From 4e158907acbfd11c58078a32d7b35e5d575c813c Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Thu, 3 Nov 2022 13:33:12 +0200 Subject: [PATCH 08/13] Remove comment --target flag conflicting flag list. --- bin/cml/comment/create.js | 1 - 1 file changed, 1 deletion(-) diff --git a/bin/cml/comment/create.js b/bin/cml/comment/create.js index 1c93af9cb..b02fef1b3 100644 --- a/bin/cml/comment/create.js +++ b/bin/cml/comment/create.js @@ -25,7 +25,6 @@ exports.options = kebabcaseKeys({ "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.', - conflicts: ['pr', 'issue', 'commitSha'], default: 'auto' }, pr: { From dc43ca0f81900d668748a8073e9f73179d3ec494 Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Thu, 3 Nov 2022 19:42:47 +0200 Subject: [PATCH 09/13] Silent default for --target flag. --- bin/cml/comment/create.js | 5 ++--- src/cml.js | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/bin/cml/comment/create.js b/bin/cml/comment/create.js index b02fef1b3..ee6d5f340 100644 --- a/bin/cml/comment/create.js +++ b/bin/cml/comment/create.js @@ -23,9 +23,8 @@ exports.options = kebabcaseKeys({ 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. " + - "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.', - default: 'auto' + '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', diff --git a/src/cml.js b/src/cml.js index e13ac5012..ee9b42f56 100755 --- a/src/cml.js +++ b/src/cml.js @@ -171,7 +171,7 @@ class CML { publishUrl, markdownFile, report: testReport, - target: commentTarget, + target: commentTarget = 'auto', watch, triggerFile } = opts; From acae963b9bbfe748f2c128896452a2aa32f04beb Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Thu, 3 Nov 2022 23:52:22 +0200 Subject: [PATCH 10/13] Fix passing of commit flag value to comment target. --- src/cml.js | 5 +---- src/commenttarget.js | 8 +------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/cml.js b/src/cml.js index ee9b42f56..b94da1ebf 100755 --- a/src/cml.js +++ b/src/cml.js @@ -176,9 +176,6 @@ class CML { triggerFile } = opts; - const commitSha = - (await this.revParse({ ref: inCommitSha })) || inCommitSha; - if (rmWatermark && update) throw new Error('watermarks are mandatory for updateable comments'); @@ -270,7 +267,7 @@ class CML { }; const target = await parseCommentTarget({ - commitSha, + inCommitSha, pr, target: commentTarget, drv diff --git a/src/commenttarget.js b/src/commenttarget.js index 227e93edc..746d60939 100644 --- a/src/commenttarget.js +++ b/src/commenttarget.js @@ -1,16 +1,10 @@ const SEPARATOR = '#'; async function parseCommentTarget(opts = {}) { - const { commitSha: commit, issue, pr, target, drv } = opts; + const { commitSha: commit, pr, target, drv } = opts; let commentTarget = target; // Handle legacy comment target flags. - if (issue) { - drv.warn( - 'cml: the --issue flag will be deprecated, please use --target="issue#123"' - ); - commentTarget = `issue#${issue}`; - } if (commit) { drv.warn( 'cml: the --commitSha flag will be deprecated, please use --target="commit#"' From c0d976124e78cfb627df0181a7a5c8806d9f03b1 Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Wed, 9 Nov 2022 11:43:41 +0200 Subject: [PATCH 11/13] Fix comment create cli param definition. --- bin/cml/comment/create.js | 5 +++-- bin/cml/comment/create.test.js | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bin/cml/comment/create.js b/bin/cml/comment/create.js index 0c6a30084..4ea9c2880 100644 --- a/bin/cml/comment/create.js +++ b/bin/cml/comment/create.js @@ -30,12 +30,13 @@ exports.options = kebabcaseKeys({ type: 'boolean', description: 'Post to an existing PR/MR associated with the specified commit', - conflicts: ['issue'] + conflicts: ['target', 'commitSha'] }, commitSha: { type: 'string', alias: 'head-sha', - description: 'Commit SHA linked to this comment' + description: 'Commit SHA linked to this comment', + conflicts: ['target', 'pr'] }, watch: { type: 'boolean', diff --git a/bin/cml/comment/create.test.js b/bin/cml/comment/create.test.js index 823d34de3..951e6ebce 100644 --- a/bin/cml/comment/create.test.js +++ b/bin/cml/comment/create.test.js @@ -19,11 +19,11 @@ describe('Comment integration tests', () => { 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 + a comment on a specific issue. 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. - [string] [default: \\"auto\\"] + [string] --pr Post to an existing PR/MR associated with the specified commit [boolean] --commit-sha, --head-sha Commit SHA linked to this comment [string] From 5d2785a505a3847119bbf9d457470642dca87383 Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Wed, 9 Nov 2022 11:53:19 +0200 Subject: [PATCH 12/13] Fix handling of commit comments. --- src/cml.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cml.js b/src/cml.js index 8b8d86788..38449b16e 100755 --- a/src/cml.js +++ b/src/cml.js @@ -299,7 +299,7 @@ class CML { }; const target = await parseCommentTarget({ - inCommitSha, + commitSha: inCommitSha, pr, target: commentTarget, drv From e7a7996b731956e891a45087a0d1ccad611ff6d7 Mon Sep 17 00:00:00 2001 From: Domas Monkus Date: Wed, 9 Nov 2022 15:34:32 +0200 Subject: [PATCH 13/13] Address review comments. --- src/cml.js | 4 ++-- src/commenttarget.js | 12 ++++++------ src/drivers/bitbucket_cloud.js | 2 +- src/drivers/gitlab.js | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/cml.js b/src/cml.js index 38449b16e..d06c42100 100755 --- a/src/cml.js +++ b/src/cml.js @@ -164,7 +164,7 @@ class CML { async commentCreate(opts = {}) { const { - commitSha: inCommitSha, + commitSha, markdownFile, pr, publish, @@ -299,7 +299,7 @@ class CML { }; const target = await parseCommentTarget({ - commitSha: inCommitSha, + commitSha, pr, target: commentTarget, drv diff --git a/src/commenttarget.js b/src/commenttarget.js index 746d60939..57d838724 100644 --- a/src/commenttarget.js +++ b/src/commenttarget.js @@ -16,7 +16,7 @@ async function parseCommentTarget(opts = {}) { commentTarget = 'pr'; } // Handle comment targets that are incomplete, e.g. 'pr' or 'commit'. - let prId; + let prNumber; let commitPr; switch (commentTarget) { case 'commit': @@ -24,16 +24,16 @@ async function parseCommentTarget(opts = {}) { 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 }; + prNumber = drv.pr; + if (prNumber) { + return { target: 'pr', prNumber: prNumber }; } // 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 }; + [prNumber] = commitPr.url.split('/').slice(-1); + return { target: 'pr', prNumber }; } // If target is 'auto', fallback to issuing commit comments. if (commentTarget === 'auto') { diff --git a/src/drivers/bitbucket_cloud.js b/src/drivers/bitbucket_cloud.js index 1e1325690..e96ebeb3c 100644 --- a/src/drivers/bitbucket_cloud.js +++ b/src/drivers/bitbucket_cloud.js @@ -548,7 +548,7 @@ class BitbucketCloud { } warn(message) { - console.error(message); + winston.warn(message); } } diff --git a/src/drivers/gitlab.js b/src/drivers/gitlab.js index bef56ca0a..af2705a7a 100644 --- a/src/drivers/gitlab.js +++ b/src/drivers/gitlab.js @@ -548,7 +548,7 @@ class Gitlab { } warn(message) { - console.error(message); + winston.warn(message); } }