Remove rework, change RegExp to stricter one

nodejs · Nov 7, 2017 · 0f48d22 · 0f48d22
1 parent 69e5512
commit 0f48d22
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 27 deletions.
diff --git a/lib/pr_checker.js b/lib/pr_checker.js
@@ -11,7 +11,7 @@ const WEEKDAY_WAIT = 48;
 const WEEKEND_WAIT = 72;
 
 const {
-  REVIEW_SOURCES: { FROM_COMMENT, FROM_REVIEW, FROM_REVIEW_COMMENT }
+  REVIEW_SOURCES: { FROM_COMMENT }
 } = require('./reviews');
 const {
   FIRST_TIME_CONTRIBUTOR, FIRST_TIMER
@@ -65,7 +65,7 @@ class PRChecker {
 
   getTSCHint(people) {
     const tsc = people
-      .filter((p) => p.review.source === FROM_REVIEW && p.reviewer.isTSC())
+      .filter((p) => p.reviewer.isTSC())
       .map((p) => p.reviewer.login);
     let hint = '';
     if (tsc.length > 0) {
@@ -77,7 +77,7 @@ class PRChecker {
 
   checkReviews() {
     const {
-      pr, logger, reviewers: { rejected, approved, commentApproved }
+      pr, logger, reviewers: { rejected, approved }
     } = this;
     let status = true;
 
@@ -95,12 +95,8 @@ class PRChecker {
       status = false;
       logger.warn(`Approvals: 0`);
     } else {
-      let notComm = approved.length;
-      approved.map((r) => {
-        if (r.review.source !== FROM_REVIEW) { notComm--; }
-      });
       let hint = this.getTSCHint(approved);
-      logger.info(`Approvals: ${notComm}${hint}`);
+      logger.info(`Approvals: ${approved.length}${hint}`);
 
       for (const { reviewer, review } of approved) {
         if (review.source === FROM_COMMENT) {
@@ -118,16 +114,6 @@ class PRChecker {
         }
       }
     }
-    if (commentApproved && commentApproved.length !== 0) {
-      let hint = this.getTSCHint(approved);
-      logger.info(`LGTM in commented review: ${commentApproved.length}${hint}`);
-      for (const { reviewer, review } of commentApproved) {
-        if (review.source === FROM_REVIEW_COMMENT) {
-          logger.info(
-            `${reviewer.getName()} approved in via LGTM in commented review`);
-        }
-      }
-    }
 
     return status;
   }

diff --git a/lib/reviews.js b/lib/reviews.js
@@ -4,7 +4,7 @@ const {
 } = require('./review_state');
 const { isCollaborator } = require('./collaborators');
 const { ascending } = require('./comp');
-const LGTM_RE = /(\W|^)lgtm(\W|$)/i;
+const LGTM_RE = /^lgtm\W?$/i;
 const FROM_REVIEW = 'review';
 const FROM_COMMENT = 'comment';
 const FROM_REVIEW_COMMENT = 'review_comment';
@@ -134,16 +134,13 @@ class ReviewAnalyzer {
     const reviewers = this.updateMapByRawReviews(ghReviews);
     const result = {
       approved: [],
-      commentApproved: [],
       rejected: []
     };
     const collaborators = this.collaborators;
     for (const [ login, review ] of reviewers) {
       const reviewer = collaborators.get(login.toLowerCase());
-      if (review.state === APPROVED) {
+      if (review.state === APPROVED || this.isApprovedInComment(review)) {
         result.approved.push({reviewer, review});
-      } else if (this.isApprovedInComment(review)) {
-        result.commentApproved.push({reviewer, review});
       } else if (review.state === CHANGES_REQUESTED) {
         result.rejected.push({ reviewer, review });
       }
@@ -165,7 +162,7 @@ class ReviewAnalyzer {
    * @returns {boolean}
    */
   hasLGTM(object, prop) {
-    return LGTM_RE.test(object[prop]);
+    return LGTM_RE.test(object[prop].trim());
   }
 }
 

diff --git a/test/fixtures/data.js b/test/fixtures/data.js
@@ -13,12 +13,10 @@ patchPrototype(rejected, 'review', Review.prototype);
 
 const allGreenReviewers = {
   approved,
-  commentApproved: [],
   rejected: []
 };
 const rejectedReviewers = {
   rejected,
-  commentApproved: [],
   approved: []
 };
 

diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js
@@ -84,7 +84,7 @@ describe('PRChecker', () => {
         ],
         info: [
           ['Rejections: 0'],
-          ['Approvals: 2'],
+          ['Approvals: 3, 1 from TSC (bar)'],
           ['Bar User(bar)) approved in via LGTM in comments']
         ],
         error: [],