fix(dev-infra): clean up usages within pullapprove tooling (#37338)

Clean up pullapprove tooling to use newly created common utils.
Additionally, use newly created logging levels rather than
verbose flagging.

PR Close #37338

dev-infra/pullapprove/cli.ts

@@ -10,11 +10,8 @@ import {verify} from './verify';
 
 /** Build the parser for the pullapprove commands. */
 export function buildPullapproveParser(localYargs: yargs.Argv) {
-  return localYargs.help()
+  return localYargs.help().strict().demandCommand().command(
-      .strict()
+      'verify', 'Verify the pullapprove config', {}, () => verify());
-      .option('verbose', {alias: ['v'], description: 'Enable verbose logging'})
-      .demandCommand()
-      .command('verify', 'Verify the pullapprove config', {}, ({verbose}) => verify(verbose));
 }
 
 if (require.main === module) {

dev-infra/pullapprove/logging.ts

@@ -10,15 +10,15 @@ import {info} from '../utils/console';
 import {PullApproveGroupResult} from './group';
 
 /** Create logs for each pullapprove group result. */
-export function logGroup(group: PullApproveGroupResult, matched = true) {
+export function logGroup(group: PullApproveGroupResult, matched = true, printMessageFn = info) {
   const conditions = matched ? group.matchedConditions : group.unmatchedConditions;
-  info.group(`[${group.groupName}]`);
+  printMessageFn.group(`[${group.groupName}]`);
   if (conditions.length) {
     conditions.forEach(matcher => {
       const count = matcher.matchedFiles.size;
-      info(`${count} ${count === 1 ? 'match' : 'matches'} - ${matcher.expression}`);
+      printMessageFn(`${count} ${count === 1 ? 'match' : 'matches'} - ${matcher.expression}`);
     });
-    info.groupEnd();
+    printMessageFn.groupEnd();
   }
 }
 

dev-infra/pullapprove/verify.ts

@@ -6,46 +6,39 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {readFileSync} from 'fs';
-import * as path from 'path';
+import {resolve} from 'path';
-import {cd, exec, set} from 'shelljs';
 
 import {getRepoBaseDir} from '../utils/config';
-import {info} from '../utils/console';
+import {debug, info} from '../utils/console';
+import {allFiles} from '../utils/repo-files';
 
 import {PullApproveGroup} from './group';
 import {logGroup, logHeader} from './logging';
 import {parsePullApproveYaml} from './parse-yaml';
 
-export function verify(verbose = false) {
+export function verify() {
-  // Exit early on shelljs errors
+  /** Full path to PullApprove config file */
-  set('-e');
+  const PULL_APPROVE_YAML_PATH = resolve(getRepoBaseDir(), '.pullapprove.yml');
-  // Full path of the angular project directory
+  /** All tracked files in the repository. */
-  const PROJECT_DIR = getRepoBaseDir();
+  const REPO_FILES = allFiles();
-  // Change to the Angular project directory
+  /** The pull approve config file. */
-  cd(PROJECT_DIR);
-  // Full path to PullApprove config file
-  const PULL_APPROVE_YAML_PATH = path.resolve(PROJECT_DIR, '.pullapprove.yml');
-  // All relative path file names in the git repo, this is retrieved using git rather
-  // that a glob so that we only get files that are checked in, ignoring things like
-  // node_modules, .bazelrc.user, etc
-  const REPO_FILES =
-      exec('git ls-files', {silent: true}).trim().split('\n').filter((_: string) => !!_);
-  // The pull approve config file.
   const pullApproveYamlRaw = readFileSync(PULL_APPROVE_YAML_PATH, 'utf8');
-  // JSON representation of the pullapprove yaml file.
+  /** JSON representation of the pullapprove yaml file. */
   const pullApprove = parsePullApproveYaml(pullApproveYamlRaw);
-  // All of the groups defined in the pullapprove yaml.
+  /** All of the groups defined in the pullapprove yaml. */
   const groups = Object.entries(pullApprove.groups).map(([groupName, group]) => {
     return new PullApproveGroup(groupName, group);
   });
-  // PullApprove groups without conditions. These are skipped in the verification
+  /**
-  // as those would always be active and cause zero unmatched files.
+   * PullApprove groups without conditions. These are skipped in the verification
+   * as those would always be active and cause zero unmatched files.
+   */
   const groupsSkipped = groups.filter(group => !group.conditions.length);
-  // PullApprove groups with conditions.
+  /** PullApprove groups with conditions. */
   const groupsWithConditions = groups.filter(group => !!group.conditions.length);
-  // Files which are matched by at least one group.
+  /** Files which are matched by at least one group. */
   const matchedFiles: string[] = [];
-  // Files which are not matched by at least one group.
+  /** Files which are not matched by at least one group. */
   const unmatchedFiles: string[] = [];
 
   // Test each file in the repo against each group for being matched.
@@ -56,10 +49,12 @@ export function verify(verbose = false) {
       unmatchedFiles.push(file);
     }
   });
-  // Results for each group
+  /** Results for each group */
   const resultsByGroup = groupsWithConditions.map(group => group.getResults());
-  // Whether all group condition lines match at least one file and all files
+  /**
-  // are matched by at least one group.
+   * Whether all group condition lines match at least one file and all files
+   * are matched by at least one group.
+   */
   const verificationSucceeded =
       resultsByGroup.every(r => !r.unmatchedCount) && !unmatchedFiles.length;
 
@@ -81,7 +76,7 @@ export function verify(verbose = false) {
    */
   logHeader('PullApprove results by file');
   info.group(`Matched Files (${matchedFiles.length} files)`);
-  verbose && matchedFiles.forEach(file => info(file));
+  matchedFiles.forEach(file => debug(file));
   info.groupEnd();
   info.group(`Unmatched Files (${unmatchedFiles.length} files)`);
   unmatchedFiles.forEach(file => info(file));
@@ -91,11 +86,11 @@ export function verify(verbose = false) {
    */
   logHeader('PullApprove results by group');
   info.group(`Groups skipped (${groupsSkipped.length} groups)`);
-  verbose && groupsSkipped.forEach(group => info(`${group.groupName}`));
+  groupsSkipped.forEach(group => debug(`${group.groupName}`));
   info.groupEnd();
   const matchedGroups = resultsByGroup.filter(group => !group.unmatchedCount);
   info.group(`Matched conditions by Group (${matchedGroups.length} groups)`);
-  verbose && matchedGroups.forEach(group => logGroup(group));
+  matchedGroups.forEach(group => logGroup(group, true, debug));
   info.groupEnd();
   const unmatchedGroups = resultsByGroup.filter(group => group.unmatchedCount);
   info.group(`Unmatched conditions by Group (${unmatchedGroups.length} groups)`);