feat(framework-core): return review number and variable renaming

README.md

@@ -65,18 +65,21 @@ and suggestions whose diff hunk is a subset of the pull request's files hunks.
  If the program terminates without exception, a timeline comment will be made with all errors or suggestions that could not be made.
 
 #### Syntax
-`reviewPullRequest(octokit, rawChanges, config [, logger])`
+`reviewPullRequest(octokit, diffContents, config [, logger])`
 
 #### Parameters
 #### `octokit`
 *octokit* <br>
 **Required.** An authenticated [octokit](https://github.com/octokit/rest.js/) instance.
 
-#### `rawChanges`
-*Map<string, RawContent> | null | undefined* <br>
-**Required.** A set of files with their respective original raw file content and the new file content. If it is null, the empty map, or undefined, a review is not made.
+#### `diffContents`
+*Map<string, FileDiffContent> | null | undefined* <br>
+**Required.** A set of files with their respective original text file content and the new file content.
+If the map is null, the empty map, or undefined, a review is not made.
+A review is also not made when forall FileDiffContent objects, f,
+f.oldContent == f.newContent
 
-**RawContent Object**
+**FileDiffContent Object**
 |      field      |     type  	|   description	|
 |---------------	|-----------	|-------------	|
 |   oldContent	|   `string`	| **Required.** The older version of a file.  |
@@ -98,6 +101,8 @@ and suggestions whose diff hunk is a subset of the pull request's files hunks.
 *[Logger](https://www.npmjs.com/package/@types/pino)* <br>
 The default logger is [Pino](https://github.com/pinojs/pino). You can plug in any logger that conforms to [Pino's interface](https://www.npmjs.com/package/@types/pino)
 
+#### returns
+returns the review number if a review was created, or null if a review was not made and an exception was not thrown.
 
 #### Exceptions
 The core-library will throw an exception if the [GitHub V3 API](https://developer.github.com/v3/) returns an error response, or if the response data format did not come back as expected. <br>

src/github-handler/comment-handler/index.ts

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import {RawContent, RepoDomain} from '../../types';
+import {FileDiffContent, RepoDomain} from '../../types';
 import {getPullRequestScope} from './get-hunk-scope-handler/';
 import {Octokit} from '@octokit/rest';
 import {getSuggestionPatches} from './raw-patch-handler';
@@ -25,15 +25,16 @@ import {logger} from '../../logger';
  * @param {RepoDomain} remote the Pull Request repository
  * @param {number} pullNumber the Pull Request number
  * @param {number} pageSize the number of files to comment on // TODO pagination
- * @param {Map<string, RawContent>} rawChanges the old and new contents of the files to suggest
+ * @param {Map<string, FileDiffContent>} diffContents the old and new contents of the files to suggest
+ * @returns the created review's id, or null if no review was made
  */
 export async function reviewPullRequest(
   octokit: Octokit,
   remote: RepoDomain,
   pullNumber: number,
   pageSize: number,
-  rawChanges: Map<string, RawContent>
-): Promise<void> {
+  diffContents: Map<string, FileDiffContent>
+): Promise<number | null> {
   try {
     const {invalidFiles, validFileLines} = await getPullRequestScope(
       octokit,
@@ -42,17 +43,18 @@ export async function reviewPullRequest(
       pageSize
     );
     const {filePatches, outOfScopeSuggestions} = getSuggestionPatches(
-      rawChanges,
+      diffContents,
       invalidFiles,
       validFileLines
     );
-    await makeInlineSuggestions(
+    const reviewNumber = await makeInlineSuggestions(
       octokit,
       filePatches,
       outOfScopeSuggestions,
       remote,
       pullNumber
     );
+    return reviewNumber;
   } catch (err) {
     logger.error('Failed to suggest');
     throw err;

src/github-handler/comment-handler/make-review-handler/upload-comments-handler.ts

@@ -73,25 +73,25 @@ export function buildReviewComments(
 
 /**
  * Make a request to GitHub to make review comments
- * @param octokit
- * @param suggestions
- * @param remote
- * @param pullNumber
+ * @param octokit an authenticated octokit instance
+ * @param suggestions code suggestions patches
+ * @param remote the repository domain
+ * @param pullNumber the pull request number to make a review on
  */
 export async function makeInlineSuggestions(
   octokit: Octokit,
   suggestions: Map<string, Patch[]>,
   outOfScopeSuggestions: Map<string, Hunk[]>,
   remote: RepoDomain,
   pullNumber: number
-): Promise<void> {
+): Promise<number | null> {
   const comments = buildReviewComments(suggestions);
   if (!comments.length) {
     logger.info('No valid suggestions to make');
   }
   if (!comments.length && !outOfScopeSuggestions.size) {
     logger.info('No suggestions were generated. Exiting...');
-    return;
+    return null;
   }
   const summaryComment = buildSummaryComment(outOfScopeSuggestions);
   if (summaryComment) {
@@ -107,15 +107,19 @@ export async function makeInlineSuggestions(
       pull_number: pullNumber,
     })
   ).data.head.sha;
-  await octokit.pulls.createReview({
-    owner: remote.owner,
-    repo: remote.repo,
-    pull_number: pullNumber,
-    commit_id: headSha,
-    event: 'COMMENT',
-    body: summaryComment,
-    headers: {accept: COMFORT_PREVIEW_HEADER},
-    // Octokit type definitions doesn't support mulitiline comments, but the GitHub API does
-    comments: (comments as unknown) as PullsCreateReviewParamsComments[],
-  });
+  const reviewNumber = (
+    await octokit.pulls.createReview({
+      owner: remote.owner,
+      repo: remote.repo,
+      pull_number: pullNumber,
+      commit_id: headSha,
+      event: 'COMMENT',
+      body: summaryComment,
+      headers: {accept: COMFORT_PREVIEW_HEADER},
+      // Octokit type definitions doesn't support mulitiline comments, but the GitHub API does
+      comments: (comments as unknown) as PullsCreateReviewParamsComments[],
+    })
+  ).data.id;
+  logger.info(`Successfully created a review on pull request: ${pullNumber}.`);
+  return reviewNumber;
 }

src/github-handler/comment-handler/raw-patch-handler/hunk-to-patch-handler.ts

@@ -12,26 +12,26 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import {Hunk, Patch, RawContent} from '../../../types';
+import {Hunk, Patch, FileDiffContent} from '../../../types';
 
 /**
  * From the each file's hunk, generate each file's hunk's old version range
  * and the new content (from the user's) to update that range with.
  * @param filesHunks a list of hunks for each file where the lines are at least 1
- * @param rawChanges
+ * @param diffContents
  * @returns patches to upload to octokit
  */
 export function generatePatches(
   filesHunks: Map<string, Hunk[]>,
-  rawChanges: Map<string, RawContent>
+  diffContents: Map<string, FileDiffContent>
 ): Map<string, Patch[]> {
   const filesPatches: Map<string, Patch[]> = new Map();
   filesHunks.forEach((hunks, fileName) => {
     const patches: Patch[] = [];
     // we expect all hunk lists to not be empty
     hunks.forEach(hunk => {
       // returns a copy of the hashmap value, then creates a new list with new substrings
-      const lines = rawChanges.get(fileName)!.newContent.split('\n');
+      const lines = diffContents.get(fileName)!.newContent.split('\n');
       // creates a new shallow-copied subarray
       // we assume the hunk new start and new end to be within the domain of the lines length
       if (

src/github-handler/comment-handler/raw-patch-handler/in-scope-hunks-handler.ts

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import {Hunk, Range, RawContent} from '../../../types';
+import {Hunk, Range, FileDiffContent} from '../../../types';
 import {getRawSuggestionHunks} from './raw-hunk-handler';
 
 /**
@@ -165,19 +165,19 @@ export function mergeOutOfScopeSuggestions(
 
 /**
  * Get the in scope (of the corresponding pull request's) hunks and files
- * @param {Map<string, RawContent>} rawChanges the raw old content and new content of a file
+ * @param {Map<string, FileDiffContent>} diffContents the old text content and new text content of a file
  * @param {string[]} invalidFiles list of invalid files
  * @param {Map<string, Range[]>} validFileLines a map of each file's in scope lines for a Pull Request
  */
 export function getValidSuggestionHunks(
-  rawChanges: Map<string, RawContent>,
+  diffContents: Map<string, FileDiffContent>,
   invalidFiles: string[],
   validFileLines: Map<string, Range[]>
 ): {
   inScopeSuggestions: Map<string, Hunk[]>;
   outOfScopeSuggestions: Map<string, Hunk[]>;
 } {
-  const totalfileHunks = getRawSuggestionHunks(rawChanges);
+  const totalfileHunks = getRawSuggestionHunks(diffContents);
   const outofScopeByFilename = getOutOfScopeByFileName(
     invalidFiles,
     totalfileHunks

src/github-handler/comment-handler/raw-patch-handler/index.ts

@@ -14,7 +14,7 @@
 
 import {generatePatches} from './hunk-to-patch-handler';
 import {getValidSuggestionHunks} from './in-scope-hunks-handler';
-import {Hunk, RawContent, Range, Patch} from '../../../types';
+import {Hunk, FileDiffContent, Range, Patch} from '../../../types';
 
 interface SuggestionPatches {
   filePatches: Map<string, Patch[]>;
@@ -25,23 +25,23 @@ interface SuggestionPatches {
  * Get the range of the old version of every file and the corresponding new text for that range
  * whose old and new contents differ, under the constraints that the file
  * is in scope for Pull Request, as well as its lines.
- * @param {Map<string, RawContent>} rawChanges the raw old content and new content of a file
+ * @param {Map<string, FileDiffContent>} diffContents the old text content and new text content of a file
  * @param {string[]} invalidFiles list of invalid files
  * @param {Map<string, Range[]>} validFileLines a map of each file's in scope lines for a Pull Request
  */
 export function getSuggestionPatches(
-  rawChanges: Map<string, RawContent>,
+  diffContents: Map<string, FileDiffContent>,
   invalidFiles: string[],
   validFileLines: Map<string, Range[]>
 ): SuggestionPatches {
   const {inScopeSuggestions, outOfScopeSuggestions} = getValidSuggestionHunks(
-    rawChanges,
+    diffContents,
     invalidFiles,
     validFileLines
   );
   const filePatches: Map<string, Patch[]> = generatePatches(
     inScopeSuggestions,
-    rawChanges
+    diffContents
   );
   return {filePatches, outOfScopeSuggestions};
 }

src/github-handler/comment-handler/raw-patch-handler/raw-hunk-handler.ts

@@ -12,27 +12,27 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import {Hunk, RawContent} from '../../../types';
+import {Hunk, FileDiffContent} from '../../../types';
 import {logger} from '../../../logger';
 import {structuredPatch, Hunk as RawHunk} from 'diff';
 
 /**
  * Given the old content and new content of a file
  * compute the diff and extract the necessary hunk data.
  * The hunk list is a list of disjoint ascending intervals.
- * @param rawContent
+ * @param fileDiffContent
  * @param fileName
  * @returns the hunks that are generated in ascending sorted order
  */
 export function generateHunks(
-  rawContent: RawContent,
+  fileDiffContent: FileDiffContent,
   fileName: string
 ): Hunk[] {
   const rawHunks: RawHunk[] = structuredPatch(
     fileName, // old file name
     fileName, // new file name
-    rawContent.oldContent,
-    rawContent.newContent
+    fileDiffContent.oldContent,
+    fileDiffContent.newContent
   ).hunks;
   const hunks: Hunk[] = rawHunks.map(rawHunk => ({
     oldStart: rawHunk.oldStart,
@@ -49,19 +49,19 @@ export function generateHunks(
  * compute the hunk for each file whose old and new contents differ.
  * Do not compute the hunk if the old content is the same as the new content.
  * The hunk list is sorted and each interval is disjoint.
- * @param {Map<string, RawContent>} rawChanges a map of the original file contents and the new file contents
+ * @param {Map<string, FileDiffContent>} diffContents a map of the original file contents and the new file contents
  * @returns the hunks for each file whose old and new contents differ
  */
 export function getRawSuggestionHunks(
-  rawChanges: Map<string, RawContent>
+  diffContents: Map<string, FileDiffContent>
 ): Map<string, Hunk[]> {
   const fileHunks: Map<string, Hunk[]> = new Map();
-  rawChanges.forEach((rawContent: RawContent, fileName: string) => {
+  diffContents.forEach((fileDiffContent: FileDiffContent, fileName: string) => {
     // if identical don't calculate the hunk and continue in the loop
-    if (rawContent.oldContent === rawContent.newContent) {
+    if (fileDiffContent.oldContent === fileDiffContent.newContent) {
       return;
     }
-    const hunks = generateHunks(rawContent, fileName);
+    const hunks = generateHunks(fileDiffContent, fileName);
     fileHunks.set(fileName, hunks);
   });
   logger.info('Parsed ranges of old and new patch');

src/index.ts

@@ -20,7 +20,7 @@ import {
   RepoDomain,
   BranchDomain,
   FileData,
-  RawContent,
+  FileDiffContent,
   CreateReviewCommentUserOptions,
 } from './types';
 export {Changes} from './types';
@@ -38,51 +38,50 @@ import * as retry from 'async-retry';
  * that they are in scope of the pull request. Outof scope suggestions are not made.
  *
  * In-scope suggestions are specifically: the suggestion for a file must correspond to a file in the remote pull request
- * and the diff hunk computed for a raw file must produce a range that is a subset of the pull request's files hunks.
+ * and the diff hunk computed for a file's contents must produce a range that is a subset of the pull request's files hunks.
  *
  * If a file is too large to load in the review, it is skipped in the suggestion phase.
  *
  * If changes are empty then the workflow will not run.
  * Rethrows an HttpError if Octokit GitHub API returns an error. HttpError Octokit access_token and client_secret headers redact all sensitive information.
  * @param octokit The authenticated octokit instance, instantiated with an access token having permissiong to create a fork on the target repository.
- * @param rawChanges A set of changes. The changes may be empty.
+ * @param diffContents A set of changes. The changes may be empty.
  * @param options The configuration for interacting with GitHub provided by the user.
  * @param loggerOption The logger instance (optional).
+ * @returns the created review's id number, or null if there are no changes to be made.
  */
 export async function reviewPullRequest(
   octokit: Octokit,
-  rawChanges: Map<string, RawContent>,
+  diffContents: Map<string, FileDiffContent>,
   options: CreateReviewCommentUserOptions,
   loggerOption?: Logger | LoggerOptions
-): Promise<void> {
+): Promise<number | null> {
   setupLogger(loggerOption);
   // if null undefined, or the empty map then no changes have been provided.
   // Do not execute GitHub workflow
   if (
-    rawChanges === null ||
-    rawChanges === undefined ||
-    rawChanges.size === 0
+    diffContents === null ||
+    diffContents === undefined ||
+    diffContents.size === 0
   ) {
     logger.info(
       'Empty changes provided. No suggestions to be made. Cancelling workflow.'
     );
-    return;
+    return null;
   }
   const gitHubConfigs = addReviewCommentsDefaults(options);
   const remote: RepoDomain = {
     owner: gitHubConfigs.owner,
     repo: gitHubConfigs.repo,
   };
-  logger.info(
-    `Successfully created a review on pull request: ${gitHubConfigs.pullNumber}.`
-  );
-  handler.reviewPullRequest(
+  const reviewNumber = await handler.reviewPullRequest(
     octokit,
     remote,
     gitHubConfigs.pullNumber,
     gitHubConfigs.pageSize,
-    rawChanges
+    diffContents
   );
+  return reviewNumber;
 }
 
 /**

src/types/index.ts

@@ -157,7 +157,7 @@ export class PatchSyntaxError extends Error {
 /**
  * The file content of the original content and the patched content
  */
-export interface RawContent {
+export interface FileDiffContent {
   readonly oldContent: string;
   readonly newContent: string;
 }
@@ -178,7 +178,7 @@ export interface Hunk {
 }
 
 /**
- * The range of a patch along with the raw file content
+ * The range of a patch along with the raw text file content
  */
 export interface Patch extends Range {
   readonly newContent: string;