Merge pull request #33 from Automattic/feature-max-comment-reached

Put out a message to reviewers if maximum number of comments have been reached

defines.php

@@ -6,7 +6,9 @@
 
 define( 'VIPGOCI_CLIENT_ID',		'automattic-vip-go-ci' );
 define( 'VIPGOCI_SYNTAX_ERROR_STR',	'PHP Syntax Errors Found' );
-define( 'VIPGOCI_GITHUB_ERROR_STR',	'GitHub API communication error');
+define( 'VIPGOCI_GITHUB_ERROR_STR',	'GitHub API communication error. ' .
+						'Please contact a human.' );
+
 define( 'VIPGOCI_GITHUB_BASE_URL',	'https://api.github.com' );
 
 define( 'VIPGOCI_INFORMATIONAL_MESSAGE',
@@ -17,6 +19,13 @@
 
 define( 'VIPGOCI_FILE_IS_APPROVED_MSG', 'File is approved in ' .
 					'hashes-to-hashes database' );
+
+define( 'VIPGOCI_REVIEW_COMMENTS_TOTAL_MAX',
+					'Total number of active review comments per ' .
+					'Pull-Request has been reached and some ' .
+					'comments might not appear as a result. ' .
+					'Please resolve some issues to see more' );
+
 /*
  * Define exit-codes
  */

github-api.php

@@ -1639,8 +1639,7 @@ function vipgoci_github_pr_generic_comment_submit(
 }
 
 /*
- * Post a generic PR comment to GitHub, reporting
- * an error.
+ * Post a generic PR comment to GitHub.
  */
 function vipgoci_github_pr_comments_error_msg(
 	$repo_owner,
@@ -1651,8 +1650,7 @@ function vipgoci_github_pr_comments_error_msg(
 	$message
 ) {
 	vipgoci_log(
-		'GitHub reported a failure, posting a ' .
-			'comment about this to the Pull-Request',
+		'Posting a comment to the Pull-Request',
 		array(
 			'repo_owner' => $repo_owner,
 			'repo_name' => $repo_name,
@@ -1676,17 +1674,10 @@ function vipgoci_github_pr_comments_error_msg(
 
 	$github_postfields = array();
 	$github_postfields['body'] =
-		'**' . VIPGOCI_GITHUB_ERROR_STR . '**' .
-		"\n\r\n\r" .
-
 		$message .
-			" (commit-ID: " . $commit_id . ")" .
+			' (commit-ID: ' . $commit_id . ').' .
 			"\n\r";
 
-	vipgoci_markdown_comment_add_pagebreak(
-		$github_postfields['body']
-	);
-
 	vipgoci_github_post_url(
 		$github_url,
 		$github_postfields,
@@ -1696,9 +1687,6 @@ function vipgoci_github_pr_comments_error_msg(
 
 /*
  * Remove any comments made by us earlier.
- *
- * FIXME: For future alterations, move comments
- * to be removed to arguments of this function.
  */
 
 function vipgoci_github_pr_comments_cleanup(
@@ -1707,6 +1695,7 @@ function vipgoci_github_pr_comments_cleanup(
 	$commit_id,
 	$github_token,
 	$branches_ignore,
+	$comments_remove,
 	$dry_run
 ) {
 	vipgoci_log(
@@ -1717,6 +1706,7 @@ function vipgoci_github_pr_comments_cleanup(
 			'repo_name' => $repo_name,
 			'commit_id' => $commit_id,
 			'branches_ignore' => $branches_ignore,
+			'comments_remove' => $comments_remove,
 			'dry_run' => $dry_run,
 		)
 	);
@@ -1764,24 +1754,19 @@ function vipgoci_github_pr_comments_cleanup(
 			 * not want to remove those. Avoid that.
 			 */
 
-			if (
-				( strpos(
+			foreach( $comments_remove as $comments_remove_item ) {
+				if ( strpos(
 					$pr_comment->body,
-					VIPGOCI_SYNTAX_ERROR_STR
-				) !== false )
-				||
-				( strpos(
-					$pr_comment->body,
-					VIPGOCI_GITHUB_ERROR_STR
-				) !== false )
-			) {
-				// Actually delete the comment
-				vipgoci_github_pr_generic_comment_delete(
-					$repo_owner,
-					$repo_name,
-					$github_token,
-					$pr_comment->id
-				);
+					$comments_remove_item
+				) !== false ) {
+					// Actually delete the comment
+					vipgoci_github_pr_generic_comment_delete(
+						$repo_owner,
+						$repo_name,
+						$github_token,
+						$pr_comment->id
+					);
+				}
 			}
 		}
 	}
@@ -2343,8 +2328,7 @@ function vipgoci_github_pr_review_submit(
 				$github_token,
 				$commit_id,
 				$pr_number,
-				'Error while communicating to the GitHub ' .
-					'API. Please contact a human.'
+				VIPGOCI_GITHUB_ERROR_STR
 			);
 		}
 	}

misc.php

@@ -1069,7 +1069,8 @@ function vipgoci_approved_files_comments_remove(
  */
 function vipgoci_github_results_filter_comments_to_max(
 	$options,
-	&$results
+	&$results,
+	&$prs_comments_maxed
 ) {
 
 	vipgoci_log(
@@ -1268,6 +1269,17 @@ function vipgoci_github_results_filter_comments_to_max(
 		);
 	}
 
+	/*
+	 * Populate '$prs_comments_maxed' which
+	 * indicates which Pull-Requests have
+	 * had number of comments posted limited.
+	 */
+	$prs_comments_maxed = array_map(
+		'is_array',
+		$comments_removed
+	);
+
+
 	vipgoci_log(
 		'Removed issue comments from array of to be submitted ' .
 			'comments to PRs due to limit constraints',
@@ -1276,6 +1288,8 @@ function vipgoci_github_results_filter_comments_to_max(
 			'comments_removed'		=> $comments_removed,
 		)
 	);
+
+	return;
 }
 
 /*

vip-go-ci.php

@@ -1094,6 +1094,11 @@ function vipgoci_run() {
 		$options['commit'],
 		$options['token'],
 		$options['branches-ignore'],
+		array(
+			VIPGOCI_SYNTAX_ERROR_STR,
+			VIPGOCI_GITHUB_ERROR_STR,
+			VIPGOCI_REVIEW_COMMENTS_TOTAL_MAX,
+		),
 		$options['dry-run']
 	);
 
@@ -1223,10 +1228,13 @@ function vipgoci_run() {
 	 * If set to zero, skip this part.
 	 */
 
-	if ( 0 !== $options['review-comments-total-max'] ) {
+	if ( $options['review-comments-total-max'] > 0 ) {
+		$prs_comments_maxed = array();
+
 		vipgoci_github_results_filter_comments_to_max(
 			$options,
-			$results
+			$results,
+			$prs_comments_maxed
 		);
 	}
 
@@ -1275,6 +1283,35 @@ function vipgoci_run() {
 		}
 	}
 
+	/*
+	 * If we reached maximum number of
+	 * comments earlier, put a message out
+	 * so people actually know it.
+	 */
+
+	if ( $options['review-comments-total-max'] > 0 ) {
+		foreach( array_keys(
+			$prs_comments_maxed
+		) as $pr_number ) {
+			vipgoci_github_pr_comments_error_msg(
+				$options['repo-owner'],
+				$options['repo-name'],
+				$options['token'],
+				$options['commit'],
+				$pr_number,
+				VIPGOCI_REVIEW_COMMENTS_TOTAL_MAX
+			);
+		}
+	}
+
+
+	/*
+	 * At this point, we have started to prepare
+	 * for shutdown and exit -- no review-critical
+	 * actions should be performed after this point.
+	 */
+
+
 	/*
 	 * Send out to IRC API any alerts
 	 * that are queued up.