From 5d1bedb5c011fcd51fbca931fd210e5454662fd2 Mon Sep 17 00:00:00 2001 From: cPhost Date: Thu, 15 Feb 2018 19:41:37 -0500 Subject: [PATCH 1/3] pr_summary: warn if the PR Author does have valid config setup This displays a warning if PR Author does not have valid config setup using `git config`. Before it use to just display as: `Author null <> (@github-user-name)` fixes: https://github.com/nodejs/node-core-utils/issues/180 --- lib/pr_summary.js | 12 +++++++-- test/fixtures/data.js | 2 ++ test/fixtures/incorrect_config_pr.json | 22 ++++++++++++++++ test/unit/pr_summary.test.js | 36 +++++++++++++++++++++++++- 4 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/incorrect_config_pr.json diff --git a/lib/pr_summary.js b/lib/pr_summary.js index b126450a..3c4fb257 100644 --- a/lib/pr_summary.js +++ b/lib/pr_summary.js @@ -34,8 +34,16 @@ class PRSummary { cli.table('Title', `${title} (#${prid})`); const authorHint = this.data.authorIsNew() ? ', first-time contributor' : ''; - cli.table('Author', - `${author.name} <${author.email}> (@${author.login}${authorHint})`); + + if (author.name && author.email) { + cli.table('Author', + `${author.name} <${author.email}> (@${author.login}${authorHint})`); + } else { + // user do not have correct `git config` setup + cli.warn('The author of the PR does not have ' + + 'correct email and name setup locally!'); + } + cli.table('Branch', `${branch}`); cli.table('Labels', `${labelStr}`); diff --git a/test/fixtures/data.js b/test/fixtures/data.js index 33364bbf..555c4856 100644 --- a/test/fixtures/data.js +++ b/test/fixtures/data.js @@ -59,6 +59,7 @@ const firstTimerPrivatePR = readJSON('first_timer_pr_with_private_email.json'); const semverMajorPR = readJSON('semver_major_pr.json'); const fixAndRefPR = readJSON('pr_with_fixes_and_refs.json'); const conflictingPR = readJSON('conflicting_pr.json'); +const incorrectConfigPR = readJSON('incorrect_config_pr.json'); const closedPR = readJSON('./closed_pr.json'); const mergedPR = readJSON('./merged_pr.json'); const readme = readFile('./README/README.md'); @@ -90,6 +91,7 @@ module.exports = { semverMajorPR, fixAndRefPR, conflictingPR, + incorrectConfigPR, readme, readmeNoTsc, readmeNoTscE, diff --git a/test/fixtures/incorrect_config_pr.json b/test/fixtures/incorrect_config_pr.json new file mode 100644 index 00000000..51e05986 --- /dev/null +++ b/test/fixtures/incorrect_config_pr.json @@ -0,0 +1,22 @@ +{ + "createdAt": "2017-10-24T11:13:43Z", + "authorAssociation": "COLLABORATOR", + "author": { + "login": "pr_author", + "email": "", + "name": null + }, + "url": "https://github.com/nodejs/node/pull/18721", + "bodyHTML": "

Fix mdn links

", + "bodyText": "Fix mdn links", + "labels": { + "nodes": [ + { + "name": "doc" + } + ] + }, + "title": "doc: fix mdn links", + "baseRefName": "master", + "headRefName": "fix-links" +} diff --git a/test/unit/pr_summary.test.js b/test/unit/pr_summary.test.js index 45915594..ca6e0045 100644 --- a/test/unit/pr_summary.test.js +++ b/test/unit/pr_summary.test.js @@ -4,7 +4,8 @@ const { oddCommits, simpleCommits, firstTimerPR, - semverMajorPR + semverMajorPR, + incorrectConfigPR } = require('../fixtures/data'); const TestCLI = require('../fixtures/test_cli'); const PRSummary = require('../../lib/pr_summary'); @@ -86,4 +87,37 @@ describe('PRSummary', () => { summary.display(); cli.assertCalledWith(expectedLogs); }); + + it('displays warning if pr author/email is not present', () => { + const cli = new TestCLI(); + const prData = { + pr: incorrectConfigPR, + commits: simpleCommits, + authorIsNew() { + return false; + } + }; + + const expectedLogs = { + log: [ + [' - doc: some changes'], + [' - Their Github Account email '] + ], + table: [ + ['Title', 'doc: fix mdn links (#16348)'], + ['Branch', 'pr_author:fix-links -> nodejs:master'], + ['Labels', 'doc'], + ['Commits', '1'], + ['Committers', '1'] + ], + warn: [ + ['The author of the PR does not have correct ' + + 'email and name setup locally!'] + ] + }; + + const summary = new PRSummary(argv, cli, prData); + summary.display(); + cli.assertCalledWith(expectedLogs); + }); }); From 7d3aa59784d5e965c0cd9a8357308accdb211702 Mon Sep 17 00:00:00 2001 From: cPhost Date: Fri, 16 Feb 2018 12:44:10 -0500 Subject: [PATCH 2/3] [squash] fix warning msg. --- lib/pr_summary.js | 6 +++--- test/unit/pr_summary.test.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/pr_summary.js b/lib/pr_summary.js index 3c4fb257..6fced93f 100644 --- a/lib/pr_summary.js +++ b/lib/pr_summary.js @@ -39,9 +39,9 @@ class PRSummary { cli.table('Author', `${author.name} <${author.email}> (@${author.login}${authorHint})`); } else { - // user do not have correct `git config` setup - cli.warn('The author of the PR does not have ' + - 'correct email and name setup locally!'); + // Unable to retrive email/name of the PR Author + cli.warn('Could not retrieve the email or name ' + + "of the PR author's from user's GitHub profile!"); } cli.table('Branch', `${branch}`); diff --git a/test/unit/pr_summary.test.js b/test/unit/pr_summary.test.js index ca6e0045..62c2ce05 100644 --- a/test/unit/pr_summary.test.js +++ b/test/unit/pr_summary.test.js @@ -111,8 +111,8 @@ describe('PRSummary', () => { ['Committers', '1'] ], warn: [ - ['The author of the PR does not have correct ' + - 'email and name setup locally!'] + ['Could not retrieve the email or name ' + + "of the PR author's from user's GitHub profile!"] ] }; From 4d5485c53e618c03a2fafc443227df1b4dab2ae5 Mon Sep 17 00:00:00 2001 From: cPhost Date: Sat, 17 Feb 2018 11:43:21 -0500 Subject: [PATCH 3/3] [squash] rename incorrectConfigPR -> emptyProfilePR --- test/fixtures/data.js | 4 ++-- .../{incorrect_config_pr.json => empty_profile_pr.json} | 0 test/unit/pr_summary.test.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename test/fixtures/{incorrect_config_pr.json => empty_profile_pr.json} (100%) diff --git a/test/fixtures/data.js b/test/fixtures/data.js index 555c4856..54d7c9f9 100644 --- a/test/fixtures/data.js +++ b/test/fixtures/data.js @@ -59,7 +59,7 @@ const firstTimerPrivatePR = readJSON('first_timer_pr_with_private_email.json'); const semverMajorPR = readJSON('semver_major_pr.json'); const fixAndRefPR = readJSON('pr_with_fixes_and_refs.json'); const conflictingPR = readJSON('conflicting_pr.json'); -const incorrectConfigPR = readJSON('incorrect_config_pr.json'); +const emptyProfilePR = readJSON('empty_profile_pr.json'); const closedPR = readJSON('./closed_pr.json'); const mergedPR = readJSON('./merged_pr.json'); const readme = readFile('./README/README.md'); @@ -91,7 +91,7 @@ module.exports = { semverMajorPR, fixAndRefPR, conflictingPR, - incorrectConfigPR, + emptyProfilePR, readme, readmeNoTsc, readmeNoTscE, diff --git a/test/fixtures/incorrect_config_pr.json b/test/fixtures/empty_profile_pr.json similarity index 100% rename from test/fixtures/incorrect_config_pr.json rename to test/fixtures/empty_profile_pr.json diff --git a/test/unit/pr_summary.test.js b/test/unit/pr_summary.test.js index 62c2ce05..46d5e3c6 100644 --- a/test/unit/pr_summary.test.js +++ b/test/unit/pr_summary.test.js @@ -5,7 +5,7 @@ const { simpleCommits, firstTimerPR, semverMajorPR, - incorrectConfigPR + emptyProfilePR } = require('../fixtures/data'); const TestCLI = require('../fixtures/test_cli'); const PRSummary = require('../../lib/pr_summary'); @@ -91,7 +91,7 @@ describe('PRSummary', () => { it('displays warning if pr author/email is not present', () => { const cli = new TestCLI(); const prData = { - pr: incorrectConfigPR, + pr: emptyProfilePR, commits: simpleCommits, authorIsNew() { return false;