Skip to content

Commit 48ef162

Browse files
authored
Add a section for how to review code more easily (rust-lang#1538)
- How to hide whitespace - Fetching PRs without having to add a new remote or copy-paste the URL of the author's fork - How to review large blocks that have moved - Suggest `git range-diff`. This section is still mostly incomplete; fixing an exact command that will work seems kinda tricky and I don't currently have time for it.
1 parent 3b35c4e commit 48ef162

File tree

3 files changed

+38
-0
lines changed

3 files changed

+38
-0
lines changed

src/git.md

+38
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,44 @@ tradeoff. The main advantage is the generally linear commit history. This
468468
greatly simplifies bisecting and makes the history and commit log much easier
469469
to follow and understand.
470470

471+
## Tips for reviewing
472+
473+
**NOTE**: This section is for *reviewing* PRs, not authoring them.
474+
475+
### Hiding whitespace
476+
477+
Github has a button for disabling whitespace changes that may be useful.
478+
You can also use `git diff -w origin/master` to view changes locally.
479+
480+
![hide whitespace](./img/github-whitespace-changes.png)
481+
482+
### Fetching PRs
483+
484+
To checkout PRs locally, you can use `git fetch upstream pull/NNNNN/head && git checkout
485+
FETCH_HEAD`.
486+
487+
You can also use github's cli tool. Github shows a button on PRs where you can copy-paste the
488+
command to check it out locally. See <https://cli.github.com/> for more info.
489+
490+
![`gh` suggestion](./img/github-cli.png)
491+
492+
### Moving large sections of code
493+
494+
Git and Github's default diff view for large moves *within* a file is quite poor; it will show each
495+
line as deleted and each line as added, forcing you to compare each line yourself. Git has an option
496+
to show moved lines in a different color:
497+
498+
```
499+
git log -p --color-moved=dimmed-zebra --color-moved-ws=allow-indentation-change
500+
```
501+
502+
See [the docs for `--color-moved`](https://git-scm.com/docs/git-diff#Documentation/git-diff.txt---color-movedltmodegt) for more info.
503+
504+
### range-diff
505+
506+
See [the relevant section for PR authors](#git-range-diff). This can be useful for comparing code
507+
that was force-pushed to make sure there are no unexpected changes.
508+
471509
## Git submodules
472510

473511
**NOTE**: submodules are a nice thing to know about, but it *isn't* an absolute

src/img/github-cli.png

26.2 KB
Loading

src/img/github-whitespace-changes.png

28.5 KB
Loading

0 commit comments

Comments
 (0)