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