Skip to content

[Internal] [Code Review] Flag code analysis issues once per converter used #21503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hramos wants to merge 3 commits into from
Closed

[Internal] [Code Review] Flag code analysis issues once per converter used #21503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6bdce91
Flag code analysis issues once per converter used
hramos Oct 5, 2018
9fdbead
Ensure analyze_pr runs only on PRs from forks
hramos Oct 5, 2018
7a7f72c
Print out review feedback locally for test purposes
hramos Oct 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 26 additions & 24 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,38 +591,38 @@ jobs:
- run:
name: Analyze Shell Scripts
command: |
if [ -n "$CIRCLE_PR_NUMBER" ]; then
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
sudo apt-get install -y shellcheck
yarn add @octokit/rest@15.10.0
echo -e "\\x1B[36mAnalyzing shell scripts\\x1B[0m"
GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" ./scripts/circleci/analyze_scripts.sh
else
echo "Skipping shell script analysis."
fi
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
sudo apt-get install -y shellcheck
yarn add @octokit/rest@15.10.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" \
GITHUB_REPO="$CIRCLE_PROJECT_REPONAME" \
GITHUB_PR_NUMBER="$CIRCLE_PR_NUMBER" \
./scripts/circleci/analyze_scripts.sh
when: always

- run:
name: Analyze Code
command: |
if [ -n "$CIRCLE_PR_NUMBER" ]; then
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"; yarn add @octokit/rest@15.10.0
echo -e "\\x1B[36mAnalyzing code\\x1B[0m"; GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" ./scripts/circleci/analyze_code.sh
else
echo "Skipping code analysis."
fi
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"; yarn add @octokit/rest@15.10.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" \
GITHUB_REPO="$CIRCLE_PROJECT_REPONAME" \
GITHUB_PR_NUMBER="$CIRCLE_PR_NUMBER" \
./scripts/circleci/analyze_code.sh
when: always

- run:
name: Analyze Pull Request
command: |
if [ -n "$CIRCLE_PR_NUMBER" ]; then
cd bots
yarn install --non-interactive --cache-folder ~/.cache/yarn
DANGER_GITHUB_API_TOKEN="$PUBLIC_PULLBOT_GITHUB_TOKEN_A""$PUBLIC_PULLBOT_GITHUB_TOKEN_B" yarn danger
else
echo "Skipping pull request analysis."
fi
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
cd bots
yarn install --non-interactive --cache-folder ~/.cache/yarn
echo -e "\\x1B[36mAnalyzing pull request\\x1B[0m"; \
DANGER_GITHUB_API_TOKEN="$PUBLIC_PULLBOT_GITHUB_TOKEN_A""$PUBLIC_PULLBOT_GITHUB_TOKEN_B" \
yarn danger
when: always
- save-cache: *save-cache-analysis

Expand Down Expand Up @@ -730,6 +730,8 @@ workflows:
tags:
only: /v[0-9]+(\.[0-9]+)*(\-rc(\.[0-9]+)?)?/

# Run code checks
# Run code checks on PRs from forks
- analyze_pr:
filters: *filter-ignore-master-stable
filters:
branches:
only: /^pull\/.*$/
4 changes: 2 additions & 2 deletions scripts/circleci/analyze_code.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run fl
# check status
STATUS=$?
if [ $STATUS == 0 ]; then
echo "Code analyzed successfully"
echo "Code analyzed successfully."
else
echo "Code analysis failed, error status $STATUS"
echo "Code analysis failed, error status $STATUS."
fi
4 changes: 2 additions & 2 deletions scripts/circleci/analyze_scripts.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ cat <(echo shellcheck; printf '%s\n' "${results[@]}" | jq .,[] | jq -s . | jq --
# check status
STATUS=$?
if [ $STATUS == 0 ]; then
echo "Shell scripts analyzed successfully"
echo "Shell scripts analyzed successfully."
else
echo "Shell script analysis failed, error status $STATUS"
echo "Shell script analysis failed, error status $STATUS."
fi
111 changes: 66 additions & 45 deletions scripts/circleci/code-analysis-bot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@

'use strict';

if (!process.env.CIRCLE_PROJECT_USERNAME) {
console.error('Missing CIRCLE_PROJECT_USERNAME. Example: facebook');
if (!process.env.GITHUB_OWNER) {
console.error('Missing GITHUB_OWNER. Example: facebook');
process.exit(1);
}
if (!process.env.CIRCLE_PROJECT_REPONAME) {
console.error('Missing CIRCLE_PROJECT_REPONAME. Example: react-native');
if (!process.env.GITHUB_REPO) {
console.error('Missing GITHUB_REPO. Example: react-native');
process.exit(1);
}

Expand Down Expand Up @@ -162,35 +162,51 @@ function getLineMapFromPatch(patchString) {
return lineMap;
}

function sendReview(owner, repo, number, commit_id, comments, convertersUsed) {
if (comments.length === 0) {
// Do not leave an empty review.
return;
}

let body = '**Code analysis results:**\n\n';
convertersUsed.forEach(converter => {
body += '* `' + converter + '` found some issues.\n';
});

const event = 'REQUEST_CHANGES';

const opts = {
owner,
repo,
number,
commit_id,
body,
event,
comments,
};
function sendReview(owner, repo, number, commit_id, body, comments) {
if (process.env.GITHUB_TOKEN) {
if (comments.length === 0) {
// Do not leave an empty review.
return;
}

octokit.pullRequests.createReview(opts, function(error, res) {
if (error) {
console.error(error);
const event = 'REQUEST_CHANGES';

const opts = {
owner,
repo,
number,
commit_id,
body,
event,
comments,
};

octokit.pullRequests.createReview(opts, function(error, res) {
if (error) {
console.error(error);
return;
}
});
} else {
if (comments.length === 0) {
console.log('No issues found.');
return;
}
});

if (process.env.CIRCLE_CI) {
console.error(
'Code analysis found issues, but the review cannot be posted to GitHub without an access token.',
);
process.exit(1);
}

let results = body + '\n';
comments.forEach(comment => {
results +=
comment.path + ':' + comment.position + ': ' + comment.body + '\n';
});
console.log(results);
}
}

function main(messages, owner, repo, number) {
Expand All @@ -199,18 +215,17 @@ function main(messages, owner, repo, number) {
return;
}

if (!process.env.GITHUB_TOKEN) {
console.error(
'Missing GITHUB_TOKEN. Example: 5fd88b964fa214c4be2b144dc5af5d486a2f8c1e',
if (process.env.GITHUB_TOKEN) {
octokit.authenticate({
type: 'oauth',
token: process.env.GITHUB_TOKEN,
});
} else {
console.log(
'Missing GITHUB_TOKEN. Example: 5fd88b964fa214c4be2b144dc5af5d486a2f8c1e. Review feedback with code analysis results will not be provided on GitHub.',
);
process.exit(1);
}

octokit.authenticate({
type: 'oauth',
token: process.env.GITHUB_TOKEN,
});

getShaFromPullRequest(owner, repo, number, sha => {
getFilesFromCommit(owner, repo, sha, files => {
let comments = [];
Expand All @@ -234,7 +249,13 @@ function main(messages, owner, repo, number) {
}); // forEach
}); // filter

sendReview(owner, repo, number, sha, comments, convertersUsed);
let body = '**Code analysis results:**\n\n';
const uniqueconvertersUsed = [...new Set(convertersUsed)];
uniqueconvertersUsed.forEach(converter => {
body += '* `' + converter + '` found some issues.\n';
});

sendReview(owner, repo, number, sha, body, comments);
}); // getFilesFromCommit
}); // getShaFromPullRequest
}
Expand Down Expand Up @@ -287,16 +308,16 @@ process.stdin.on('end', function() {
delete messages[absolutePath];
}

const owner = process.env.CIRCLE_PROJECT_USERNAME;
const repo = process.env.CIRCLE_PROJECT_REPONAME;
const owner = process.env.GITHUB_OWNER;
const repo = process.env.GITHUB_REPO;

if (!process.env.CIRCLE_PR_NUMBER) {
console.error('Missing CIRCLE_PR_NUMBER. Example: 4687');
if (!process.env.GITHUB_PR_NUMBER) {
console.error('Missing GITHUB_PR_NUMBER. Example: 4687');
// for master branch, don't throw an error
process.exit(0);
}

const number = process.env.CIRCLE_PR_NUMBER;
const number = process.env.GITHUB_PR_NUMBER;

// intentional lint warning to make sure that the bot is working :)
main(messages, owner, repo, number);
Expand Down