From df5c572cded5a1b96da0894d3e3b15019116c594 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 15 Aug 2020 14:14:03 -0700 Subject: [PATCH] fix: allow opt-out of Fixes/Refs metadata (#474) * fix: allow opt-out of Fixes/Refs metadata * Add metadata gen test --- components/git/land.js | 10 ++++++++-- components/metadata.js | 4 ++-- docs/git-node.md | 2 ++ lib/metadata_gen.js | 18 +++++++++++++----- test/unit/metadata_gen.test.js | 12 ++++++++++++ 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/components/git/land.js b/components/git/land.js index 0c5bfc76..e39ac83d 100644 --- a/components/git/land.js +++ b/components/git/land.js @@ -42,6 +42,11 @@ const landOptions = { describe: 'Land a backport PR onto a staging branch', default: false, type: 'boolean' + }, + skipRefs: { + describe: 'Prevent adding Fixes and Refs information to commit metadata', + default: false, + type: 'boolean' } }; @@ -89,7 +94,8 @@ function handler(argv) { const provided = []; for (const type of Object.keys(landOptions)) { - if (type === 'yes') continue; // --yes is not an action + // --yes and --skipRefs are not actions. + if (type === 'yes' || type === 'skipRefs') continue; if (argv[type]) { provided.push(type); } @@ -160,7 +166,7 @@ async function main(state, argv, cli, req, dir) { return; } session = new LandingSession(cli, req, dir, argv.prid, argv.backport); - const metadata = await getMetadata(session.argv, cli); + const metadata = await getMetadata(session.argv, argv.skipRefs, cli); if (argv.backport) { const split = metadata.metadata.split('\n')[0]; if (split === 'PR-URL: ') { diff --git a/components/metadata.js b/components/metadata.js index 499bd99f..4f7945e3 100644 --- a/components/metadata.js +++ b/components/metadata.js @@ -9,7 +9,7 @@ const MetadataGenerator = require('../lib/metadata_gen'); const fs = require('fs'); -module.exports = async function getMetadata(argv, cli) { +module.exports = async function getMetadata(argv, skipRefs, cli) { const credentials = await auth({ github: true, jenkins: true @@ -23,7 +23,7 @@ module.exports = async function getMetadata(argv, cli) { cli.separator('PR info'); summary.display(); - const metadata = new MetadataGenerator(data).getMetadata(); + const metadata = new MetadataGenerator({ skipRefs, ...data }).getMetadata(); if (!process.stdout.isTTY) { process.stdout.write(metadata); } diff --git a/docs/git-node.md b/docs/git-node.md index e0291e87..962e6fe9 100644 --- a/docs/git-node.md +++ b/docs/git-node.md @@ -47,6 +47,8 @@ Options: non-interactively. If an undesirable situation occurs, such as a pull request or commit check fails, then git node land will abort. [boolean] [default: false] + --skipRefs Prevent Fixes and Refs information from being added to commit + metadata [boolean] [default: false] Examples: diff --git a/lib/metadata_gen.js b/lib/metadata_gen.js index a22b3a88..e2a02d78 100644 --- a/lib/metadata_gen.js +++ b/lib/metadata_gen.js @@ -10,8 +10,9 @@ class MetadataGenerator { * @param {PRData} data */ constructor(data) { - const { owner, repo, pr, reviewers, argv } = data; + const { owner, repo, pr, reviewers, argv, skipRefs } = data; this.owner = owner; + this.skipRefs = skipRefs; this.repo = repo; this.pr = pr; this.reviewers = reviewers; @@ -33,10 +34,17 @@ class MetadataGenerator { const fixes = parser.getFixes(); const refs = parser.getRefs(); const altPrUrl = parser.getAltPrUrl(); - const meta = [ - ...fixes.map((fix) => `Fixes: ${fix}`), - ...refs.map((ref) => `Refs: ${ref}`) - ]; + + const meta = []; + + // If there are multiple commits in a PR, we may not want to add + // Fixes/Refs metadata to all of them. + if (!this.skipRefs) { + // Map all issues fixed by the commit(s) in this PR. + meta.push(...fixes.map((fix) => `Fixes: ${fix}`)); + // Map all issues referenced by the commit(s) in this PR. + meta.push(...refs.map((ref) => `Refs: ${ref}`)); + } const backport = this.argv ? this.argv.backport : undefined; if (backport) { meta.unshift(`Backport-PR-URL: ${prUrl}`); diff --git a/test/unit/metadata_gen.test.js b/test/unit/metadata_gen.test.js index df7a05c1..d8dbede9 100644 --- a/test/unit/metadata_gen.test.js +++ b/test/unit/metadata_gen.test.js @@ -52,6 +52,12 @@ Backport-PR-URL: https://github.com/nodejs/node/pull/30072 Fixes: https://github.com/nodejs/build/issues/1961 Refs: https://github.com/nodejs/node/commit/53ca0b9ae145c430842bf78e553e3b6cbd2823aa#commitcomment-35494896 `; +const skipRefsExpected = `PR-URL: https://github.com/nodejs/node/pull/16438 +Reviewed-By: Foo User +Reviewed-By: Quux User +Reviewed-By: Baz User +Reviewed-By: Bar User +`; describe('MetadataGenerator', () => { it('should generate metadata properly', () => { @@ -68,4 +74,10 @@ describe('MetadataGenerator', () => { const backportResults = new MetadataGenerator(backportData).getMetadata(); assert.strictEqual(backportExpected, backportResults); }); + + it('should skip adding Fixes/Refs metadata when --skipRefs is passed', () => { + const data = { skipRefs: true, ...crossData }; + const backportResults = new MetadataGenerator(data).getMetadata(); + assert.strictEqual(skipRefsExpected, backportResults); + }); });