feat(cli): add command for reviewing a pr

src/bin/code-suggester.ts

@@ -15,14 +15,14 @@
 // limitations under the License.
 
 import * as yargs from 'yargs';
-import {CREATE_PR_COMMAND, main} from './workflow';
+import {CREATE_PR_COMMAND, REVIEW_PR_COMMAND, main} from './workflow';
 import {logger} from '../logger';
 
 // tslint:disable:no-unused-expression
 // yargs actually is a used expression. TS-lint does not detect it.
 yargs
-  .scriptName('code-suggest')
-  .usage('$0 pr [args]')
+  .scriptName('code-suggester')
+  .usage('$0 <command> [args]')
   .command(CREATE_PR_COMMAND, 'Create a new pull request', {
     'upstream-repo': {
       alias: 'r',
@@ -94,6 +94,32 @@ yargs
       type: 'boolean',
     },
   })
+  .command(REVIEW_PR_COMMAND, 'Review an open pull request', {
+    'upstream-repo': {
+      alias: 'r',
+      demandOption: true,
+      describe: 'Required. The repository to create the fork off of.',
+      type: 'string',
+    },
+    'upstream-owner': {
+      alias: 'o',
+      demandOption: true,
+      describe: 'Required. The owner of the upstream repository.',
+      type: 'string',
+    },
+    'pull-number': {
+      alias: 'p',
+      demandOption: true,
+      describe: 'Required. The pull request number to comment on.',
+      type: 'number',
+    },
+    'git-dir': {
+      describe:
+        'Required. The location of any un-tracked changes that should be made into pull request comments. Files in the .gitignore are ignored.',
+      type: 'string',
+      demandOption: true,
+    },
+  })
   .check(argv => {
     for (const key in argv) {
       if (typeof argv[key] === 'string' && !argv[key]) {

src/bin/handle-git-dir-change.ts

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 import {execSync} from 'child_process';
-import {Changes, FileMode, FileData} from '../types';
+import {Changes, FileMode, FileData, FileDiffContent} from '../types';
 import {logger} from '../logger';
 import {readFile} from 'fs';
 import * as path from 'path';
@@ -137,6 +137,10 @@ export function getGitFileData(
   });
 }
 
+function getFileContentsAtHead(gitRootDir: string, filePath: string): string {
+  return execSync(`git show HEAD:${filePath}`, {cwd: gitRootDir}).toString();
+}
+
 /**
  * Get all the diffs using `git diff` of a git directory.
  * Errors if the git directory provided is not a git directory.
@@ -151,6 +155,7 @@ export function getAllDiffs(gitRootDir: string): string[] {
     .toString() // strictly return buffer for mocking purposes. sinon ts doesn't infer {encoding: 'utf-8'}
     .trimRight() // remove the trailing new line
     .split('\n');
+  execSync('git reset .', {cwd: gitRootDir});
   return diffs;
 }
 
@@ -219,3 +224,64 @@ export function getChanges(dir: string): Promise<Changes> {
     throw err;
   }
 }
+
+/**
+ * Get the git changes of the current project asynchronously.
+ * Rejects if any of the files fails to load (if not deleted),
+ * or if there is a git diff parse error
+ * @param {string[]} diffs the git diff raw output (which only shows relative paths)
+ * @param {string} gitDir the root of the local GitHub repository
+ * @returns {Promise<Changes>} the changeset
+ */
+export async function parseDiffContents(
+  diffs: string[],
+  gitDir: string
+): Promise<Map<string, FileDiffContent>> {
+  try {
+    // get updated file contents
+    const changes: Map<string, FileDiffContent> = new Map();
+    const changePromises: Array<Promise<GitFileData>> = [];
+    for (let i = 0; i < diffs.length; i++) {
+      // TODO - handle memory constraint
+      changePromises.push(getGitFileData(gitDir, diffs[i]));
+    }
+    const gitFileDatas = await Promise.all(changePromises);
+    for (let i = 0; i < gitFileDatas.length; i++) {
+      const gitfileData = gitFileDatas[i];
+      const fileDiffContent: FileDiffContent = {
+        oldContent: getFileContentsAtHead(gitDir, gitfileData.path),
+        newContent: gitfileData.fileData.content!,
+      };
+      changes.set(gitfileData.path, fileDiffContent);
+    }
+    return changes;
+  } catch (err) {
+    logger.error('Error parsing git changes');
+    throw err;
+  }
+}
+
+/**
+ * Get the git changes of the current project asynchronously.
+ * Rejects if any of the files fails to load (if not deleted),
+ * or if there is a git diff parse error
+ * @param {string[]} diffs the git diff raw output (which only shows relative paths)
+ * @param {string} gitDir the root of the local GitHub repository
+ * @returns {Promise<Changes>} the changeset
+ */
+export function getDiffContents(
+  dir: string
+): Promise<Map<string, FileDiffContent>> {
+  try {
+    validateGitInstalled();
+    const absoluteDir = resolvePath(dir);
+    const gitRootDir = findRepoRoot(absoluteDir);
+    const diffs = getAllDiffs(gitRootDir);
+    return parseDiffContents(diffs, gitRootDir);
+  } catch (err) {
+    if (!(err instanceof InstallationError)) {
+      logger.error('Error loadng git changes.');
+    }
+    throw err;
+  }
+}

src/bin/workflow.ts

@@ -1,11 +1,29 @@
-import {Changes, CreatePullRequestUserOptions} from '../types';
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import {
+  CreatePullRequestUserOptions,
+  CreateReviewCommentUserOptions,
+} from '../types';
 import {Octokit} from '@octokit/rest';
 import * as git from './handle-git-dir-change';
-import {createPullRequest} from '../';
+import {createPullRequest, reviewPullRequest} from '../';
 import {logger, setupLogger} from '../logger';
 import * as yargs from 'yargs';
 
 export const CREATE_PR_COMMAND = 'pr';
+export const REVIEW_PR_COMMAND = 'review';
 
 /**
  * map yargs to user pull request otions
@@ -25,22 +43,48 @@ export function coerceUserCreatePullRequestOptions(): CreatePullRequestUserOptio
   };
 }
 
+/**
+ * map yargs to user pull request otions
+ */
+export function coerceUserCreateReviewRequestOptions(): CreateReviewCommentUserOptions {
+  return {
+    repo: yargs.argv.upstreamRepo as string,
+    owner: yargs.argv.upstreamOwner as string,
+    pullNumber: yargs.argv.pullNumber as number,
+  };
+}
+
+async function createCommand() {
+  const options = coerceUserCreatePullRequestOptions();
+  const changes = await git.getChanges(yargs.argv['git-dir'] as string);
+  const octokit = new Octokit({auth: process.env.ACCESS_TOKEN});
+  await createPullRequest(octokit, changes, options, logger);
+}
+
+async function reviewCommand() {
+  const reviewOptions = coerceUserCreateReviewRequestOptions();
+  const diffContents = await git.getDiffContents(
+    yargs.argv['git-dir'] as string
+  );
+  const octokit = new Octokit({auth: process.env.ACCESS_TOKEN});
+  await reviewPullRequest(octokit, diffContents, reviewOptions, logger);
+}
+
 /**
  * main workflow entrance
  */
 export async function main() {
   try {
     setupLogger();
-    const options = coerceUserCreatePullRequestOptions();
     if (!process.env.ACCESS_TOKEN) {
       throw Error('The ACCESS_TOKEN should not be undefined');
     }
-    const octokit = new Octokit({auth: process.env.ACCESS_TOKEN});
-    let changes: Changes;
     switch (yargs.argv._[0]) {
       case CREATE_PR_COMMAND:
-        changes = await git.getChanges(yargs.argv['git-dir'] as string);
-        await createPullRequest(octokit, changes, options, logger);
+        createCommand();
+        break;
+      case REVIEW_PR_COMMAND:
+        reviewCommand();
         break;
       default:
         // yargs should have caught this.

test/cli.ts

@@ -50,7 +50,7 @@ describe('main', () => {
       .value({...process.env, ACCESS_TOKEN: '123121312'});
     sandbox
       .stub(yargs, 'argv')
-      .value({...yargs.argv, _: ['pr'], 'git-dir': 'some/dir'});
+      .value({...yargs.argv, _: ['unknown-command'], 'git-dir': 'some/dir'});
     const stubHelperHandlers = {
       getChanges: () => {
         return new Promise((resolve, reject) => {