A better way to do a code-review of a PR is to do it in your IDE. Here are two scripts which allow you to perform the review and create local changes which can be appended to the PR.
Run this command to load the changes into your local repository where your IDE is running.
$ ./scripts/github/review-pr 24623
This will result in output:
Already on 'master' Your branch is up to date with 'origin/master'. Fetching pull request #24623 with 1 SHA(s) into branch range: pr/24623_base..pr/24623_top ====================================================================================== cef93a51b (pr/24623_top) ci: scripts to review PRs locally ====================================================================================== Switched to a new branch 'pr/24623' On branch pr/24623 Untracked files: (use "git add <file>..." to include in what will be committed) docs/PR_REVIEW.md scripts/github/push-pr scripts/github/review-pr nothing added to commit but untracked files present (use "git add" to track)
Note that the script created
pr/24623_base branches which denote SHAs where the PR start and end.
cef93a51b (pr/24623_top) ci: scripts to review PRs locally 637805a0c (pr/24623_base) docs: update `lowercase` pipe example in "AngularJS to Angular" guide (#24588)
pr/24623_base makes it convenient to refer to different SHAs in PR when rebasing or reseting.
Because the script has reset the
HEAD of the PR the changes show up as unstaged files.
$ git status On branch pr/24623 Untracked files: (use "git add <file>..." to include in what will be committed) docs/PR_REVIEW.md scripts/github/push-pr scripts/github/review-pr nothing added to commit but untracked files present (use "git add" to track)
Use your IDE to review the untracked files as needed. A good trick is to use your IDE to stage the files which were already reviewed. When all files are staged the review is done.
At any point you can edit any line in the repository. The idea is to create edits locally and push them to the PR later. This is useful because it is often times easier to make minor changes locally than to request the PR author to change and repush through a comment (often times the comment is larger than the change.)
Example of a local edit.
echo "# here is a change" >> docs/PR_REVIEW.md
Since the HEAD has been reset to
pr/24623_base so that changes show up in
git status we have to reverse the reset to only see our local changes. To do that reset the
$ git reset pr/24623_top
Doing so will remove all PR changes and only leave your local modifications which you have done. You can verify by running
git status and
git diff to see only your changes (PR changes have been removed.)
$ git status On branch pr/24623 Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git checkout -- <file>..." to discard changes in working directory) modified: docs/PR_REVIEW.md no changes added to commit (use "git add" and/or "git commit -a")
$ git diff diff --git a/docs/PR_REVIEW.md b/docs/PR_REVIEW.md index 184b5aeca..83517fbe0 100644 --- a/docs/PR_REVIEW.md +++ b/docs/PR_REVIEW.md @@ -8,4 +8,4 @@ A better way to do code review of the PR is to do it in your IDE. Here are two s existing text - \ No newline at end of file +# here is a change
Next step is to turn your local changes into a
fixup! commit. Run
git commit --all --fixup HEAD to create a
NOTE: If you added new files they must be added using
git add . or they will not be picked up by the
git commit --all flag.
$ git commit --all --fixup HEAD [pr/24623 45ae87ce4] fixup! ci: scripts to review PRs locally 1 file changed, 1 insertion(+), 1 deletion(-)
You can verify that the
fixup! commit with your local modifications was created.
$ git log --oneline 45ae87ce4 (HEAD -> pr/24623) fixup! ci: scripts to review PRs locally cef93a51b (pr/24623_top) ci: scripts to review PRs locally
The last step is to push your local changes back into the PR. Use
./scripts/github/push-pr script for that.
$ ./scripts/github/push-pr Assuming PR #24623 >>> git push firstname.lastname@example.org:mhevery/angular.git HEAD:review_pr_script Counting objects: 4, done. Delta compression using up to 8 threads. Compressing objects: 100% (4/4), done. Writing objects: 100% (4/4), 392 bytes | 392.00 KiB/s, done. Total 4 (delta 3), reused 0 (delta 0) remote: Resolving deltas: 100% (3/3), completed with 3 local objects. To github.com:mhevery/angular.git cef93a51b..45ae87ce4 HEAD -> review_pr_script
NOTE: Notice that we did not have to specify the PR number since the script can guess it from the branch name.
If you visit https://github.com/angular/angular/pull/24623/commits you will see that your
fixup! commit has been added to the PR. This greatly simplifies the work for many minor changes to the PR.