156 lines
5.1 KiB
Markdown
156 lines
5.1 KiB
Markdown
# PR Review
|
|
|
|
## Tools
|
|
|
|
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.
|
|
|
|
### 1. Loading 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_top` and `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)
|
|
```
|
|
|
|
Knowing `pr/24623_top` and `pr/24623_base` makes it convenient to refer to different SHAs in PR when rebasing or reseting.
|
|
|
|
### 2. Review PR
|
|
|
|
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.
|
|
|
|
### 3. Creating Edits
|
|
|
|
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
|
|
```
|
|
|
|
### 4. Creating a Commit From Local Edits
|
|
|
|
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 `HEAD` to `pr/24623_top`.
|
|
|
|
```
|
|
$ 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 `fixup!` commit.
|
|
|
|
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
|
|
```
|
|
|
|
### 5. Pushing local edits back to the PR
|
|
|
|
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 git@github.com: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.
|
|
|