Fix code analysis bot failure to post lint warnings on pull requests

Summary:
The code analysis script takes the results of `eslint .` and filters out any messages for filepaths outside of what is modified in a given pull request. This diff fixes an issue where the bot will fail to post warnings if a pull request contains multiple commits, where the most recent commit is a rebase (e.g. 63c00f20a7 in https://github.com/facebook/react-native/pull/22470). This happens because the script looks for files changed in the most recent commit in a PR.

In this diff, we switch to a new GitHub API that returns the list of all files changed by a PR, obviating the need to go through individual commits in a PR to look for changed files.

Reviewed By: TheSavior

Differential Revision: D13324154

fbshipit-source-id: f9f50028439d1969b0feea65f0b3e8bf75ac1a33
This commit is contained in:
Héctor Ramos 2018-12-05 13:12:50 -08:00 committed by Facebook Github Bot
parent 4dea677b4f
commit 44878ea9bc
2 changed files with 15 additions and 17 deletions

View File

@ -574,7 +574,7 @@ jobs:
command: |
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
sudo apt-get install -y shellcheck
yarn add @octokit/rest@15.10.0
yarn add @octokit/rest@15.18.0
echo -e "\\x1B[36mAnalyzing shell scripts\\x1B[0m"; \
GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" \
GITHUB_OWNER="$CIRCLE_PROJECT_USERNAME" \
@ -586,7 +586,7 @@ jobs:
- run:
name: Analyze Code
command: |
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"; yarn add @octokit/rest@15.10.0
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"; yarn add @octokit/rest@15.18.0
echo -e "\\x1B[36mAnalyzing code\\x1B[0m"; \
GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" \
GITHUB_OWNER="$CIRCLE_PROJECT_USERNAME" \

View File

@ -118,19 +118,17 @@ function getShaFromPullRequest(owner, repo, number, callback) {
});
}
function getFilesFromCommit(owner, repo, sha, callback) {
octokit.repos.getCommit({owner, repo, sha}, (error, res) => {
if (error) {
console.error(error);
return;
}
// A merge commit should not have any new changes to report
if (res.parents && res.parents.length > 1) {
return;
}
callback(res.data.files);
});
function getFilesFromPullRequest(owner, repo, number, callback) {
octokit.pullRequests.listFiles(
{owner, repo, number, per_page: 100},
(error, res) => {
if (error) {
console.error(error);
return;
}
callback(res.data);
},
);
}
/**
@ -227,7 +225,7 @@ function main(messages, owner, repo, number) {
}
getShaFromPullRequest(owner, repo, number, sha => {
getFilesFromCommit(owner, repo, sha, files => {
getFilesFromPullRequest(owner, repo, number, files => {
let comments = [];
let convertersUsed = [];
files.filter(file => messages[file.filename]).forEach(file => {
@ -256,7 +254,7 @@ function main(messages, owner, repo, number) {
});
sendReview(owner, repo, number, sha, body, comments);
}); // getFilesFromCommit
}); // getFilesFromPullRequest
}); // getShaFromPullRequest
}