From c652c5b1c8445825e6d7a8074f89f618daaeeaee Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 23 Nov 2017 23:01:58 +0800 Subject: [PATCH 1/2] pr_checker: ignore private emails --- lib/pr_checker.js | 7 ++++++ test/fixtures/data.js | 2 ++ .../first_timer_pr_with_private_email.json | 24 +++++++++++++++++++ test/unit/pr_checker.test.js | 23 ++++++++++++++++++ 4 files changed, 56 insertions(+) create mode 100644 test/fixtures/first_timer_pr_with_private_email.json diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 68b3db3a..229fd7df 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -263,6 +263,13 @@ class PRChecker { isOddAuthor(commit) { const { pr, collaboratorEmails } = this; + + // They have turned on the private email feature, can't really check + // anything, GitHub should know how to link that, see nodejs/node#15489 + if (!pr.author.email) { + return false; + } + // If they have added the alternative email to their account, // commit.authoredByCommitter should be set to true by Github if (commit.authoredByCommitter) { diff --git a/test/fixtures/data.js b/test/fixtures/data.js index 5141dfd3..436670aa 100644 --- a/test/fixtures/data.js +++ b/test/fixtures/data.js @@ -55,6 +55,7 @@ const collaborators = new Map( ); const firstTimerPR = readJSON('first_timer_pr.json'); +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'); @@ -83,6 +84,7 @@ module.exports = { mulipleCommitsAfterCi, collaborators, firstTimerPR, + firstTimerPrivatePR, semverMajorPR, fixAndRefPR, conflictingPR, diff --git a/test/fixtures/first_timer_pr_with_private_email.json b/test/fixtures/first_timer_pr_with_private_email.json new file mode 100644 index 00000000..5e31f41c --- /dev/null +++ b/test/fixtures/first_timer_pr_with_private_email.json @@ -0,0 +1,24 @@ +{ + "createdAt": "2017-10-24T11:13:43Z", + "authorAssociation": "FIRST_TIMER", + "author": { + "login": "pr_author", + "email": "" + }, + "url": "https://github.com/nodejs/node/pull/16438", + "bodyHTML": "

Awesome changes

", + "bodyText": "Awesome changes", + "labels": { + "nodes": [ + { + "name": "test" + }, + { + "name": "doc" + } + ] + }, + "title": "test: awesome changes", + "baseRefName": "master", + "headRefName": "awesome-changes" +} diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 8109d67e..51e85c7b 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -23,6 +23,7 @@ const { mulipleCommitsAfterCi, collaborators, firstTimerPR, + firstTimerPrivatePR, semverMajorPR, conflictingPR } = require('../fixtures/data'); @@ -436,6 +437,28 @@ describe('PRChecker', () => { assert(!status); cli.assertCalledWith(expectedLogs); }); + + it('should skip checking odd commits for first timers ' + + 'with private emails', () => { + const cli = new TestCLI(); + + const expectedLogs = {}; + + const options = { + pr: firstTimerPrivatePR, + reviewers: allGreenReviewers, + comments: commentsWithLGTM, + reviews: approvingReviews, + commits: oddCommits, + collaborators + }; + const checker = new PRChecker(cli, options, argv); + + assert(checker.authorIsNew()); + const status = checker.checkAuthor(); + assert(status); + cli.assertCalledWith(expectedLogs); + }); }); describe('checkCommitsAfterReview', () => { From 2f5773e88e7e02452faa6ce50aaed00e46db2d08 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 23 Nov 2017 23:11:09 +0800 Subject: [PATCH 2/2] pr_checker: show the pr author email and their status --- lib/pr_checker.js | 4 ++-- test/unit/pr_checker.test.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 229fd7df..d693c959 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -247,8 +247,8 @@ class PRChecker { return true; } - const prAuthor = pr.author.login; - cli.warn(`PR author is: @${prAuthor}`); + const prAuthor = `${pr.author.login}(${pr.author.email})`; + cli.warn(`PR author is a new contributor: @${prAuthor}`); for (const c of oddCommits) { const { oid, author } = c.commit; const hash = oid.slice(0, 7); diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 51e85c7b..aa6caacc 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -416,7 +416,7 @@ describe('PRChecker', () => { const expectedLogs = { warn: [ - ['PR author is: @pr_author'], + ['PR author is a new contributor: @pr_author(pr_author@example.com)'], ['- commit e3ad7c7 is authored by test@example.com'], ['- commit da39a3e is authored by test@example.com'] ]