angular-cn/docs/PR_REVIEW.md

155 lines
5.1 KiB
Markdown
Raw Permalink Normal View History

# 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 resetting.
### 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.