Skip to content

Commit

Permalink
Actually remove reviewers instead of just removing it from the Gerald…
Browse files Browse the repository at this point in the history
… comment (#37)

## Summary:
In the past, if someone commented `#RemoveMe` on a pull request, it would remove them from the Gerald megacomment and prevent Gerald from readding them as reviewers in the future, but it doesn't actually remove them as a reviewer. This PR makes it so that Gerald will actually make a request to remove reviewers that have commented `#RemoveMe`.

Issue: WEB-2758

## Test plan:
See (#35), where Kevin B. was requested for review, and after commenting `#RemoveMe`, his review request was removed.

Author: yipstanley

Reviewers: jeresig

Required Reviewers: 

Approved by: jeresig

Checks: ✅ Flow Coverage, ✅ Jest Coverage, ✅ Jest, ✅ Eslint, ✅ autofix, ✅ lint_and_unit, ✅ build_index

Pull request URL: #37
  • Loading branch information
yipstanley committed Aug 12, 2020
1 parent 4180f81 commit 7c00a33
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/runOnPullRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
parseExistingComments,
getFilteredLists,
makeCommentBody,
maybeRemoveReviewRequests,
} from './utils';
import {execCmd} from './execCmd';
import {ownerAndRepo, context, extraPermGithub} from './setup';
Expand Down Expand Up @@ -100,6 +101,11 @@ export const runOnPullRequest = async () => {
removedJustNames,
);

await maybeRemoveReviewRequests(
removedJustNames,
{...ownerAndRepo, pull_number: context.issue.number},
extraPermGithub,
);
await extraPermGithub.pulls.createReviewRequest({
...ownerAndRepo,
pull_number: context.issue.number,
Expand Down
33 changes: 32 additions & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// @flow

import {type Octokit$IssuesListCommentsResponseItem, type Octokit$Response} from '@octokit/rest';
import {
type Octokit,
type Octokit$IssuesListCommentsResponseItem,
type Octokit$Response,
} from '@octokit/rest';
import fs from 'fs';
import fg from 'fast-glob'; // flow-uncovered-line

Expand Down Expand Up @@ -75,6 +79,33 @@ const filterIgnoreFiles = (fileContents: string): Array<string> => {
});
};

/**
* @desc If any of the usernames in removedJustNames are reviewers, we should also
* remove them as a reviewer.
*
* @param removedJustNames - Just the usernames (not including @) of the people
* who have requeseted to be removed.
* @param params - The owner, repo, and pull_number parameters for Octokit requests.
* They can't be imported, because that would make this file really difficult to test.
* @param githubClient - The Octokit client that we can make calls on. We also can't import this.
*/
export const maybeRemoveReviewRequests = async (
removedJustNames: Array<string>,
params: {owner: string, repo: string, pull_number: number},
githubClient: Octokit,
) => {
const {data: reviewRequests} = await githubClient.pulls.listReviewRequests({...params});
const toRemove = reviewRequests.users
.filter(user => removedJustNames.includes(user.login))
.map(user => user.login);
if (toRemove.length) {
await githubClient.pulls.deleteReviewRequest({
...params,
reviewers: toRemove,
});
}
};

/**
* @desc Read ./.geraldignore and ./.gitignore if they exist.
* Split the files by newlines to serve as the list of files/directories to
Expand Down

0 comments on commit 7c00a33

Please sign in to comment.