Introduction

prr is a tool that brings mailing list style code reviews to Github PRs. This means offline reviews and a file oriented interface, more or less.

To that end, prr introduces a new workflow for reviewing PRs:

1. Download the PR into a "review file" on your filesystem
2. Mark up the review file using your favorite text editor
3. Submit the review at your convenience

The tool was born of frustration from using the point-and-click editor text boxes on PRs. I happen to do a lot of code review and tabbing to and from the browser to cross reference code from the changes was driving me nuts.

Installation

Installation has two steps:

1. Install the prr binary
2. Acquire a token from Github
3. Give the token to prr

The following sub-chapters provide more details.

Install binary

There are multiple ways to install prr CLI tool. Choose any one of the methods below that best suit your needs.

Build from source

To build prr from source, you will first need to install Rust and Cargo. Follow the instructions on the Rust installation page.

Once you have installed Rust, the following command can be used to build and install prr:

cargo install prr

This will automatically download prr from crates.io, build it, and install it in Cargo's global binary directory (~/.cargo/bin/ by default).

To uninstall, run the command:

cargo uninstall prr

Install using Homebrew

To install prr using Homebrew, first follow Homebrew install instructions.

Once Homebrew is installed, run:

brew install prr

Acquire Github token

prr needs a token so it can make Github API calls on your behalf.

Create the token by going to:

Settings -> Developer settings -> Personal access tokens -> Generate new token

and give the token repo permissions. Keep the newly generated token handy for the next step.

Configure prr

First, create the config directory:

mkdir -p ~/.config/prr

Next, create a basic global config:

cat << EOF > ~/.config/prr/config.toml
[prr]
token = "$YOUR_PAT_FROM_LAST_STEP"
workdir = "/home/dxu/dev/review"
EOF

Note workdir can be any directory. (You don't have to use my unix name)

Tutorial

This tutorial shows you how to create a review against prr-test-repo#6. Feel free to review it!

We assume you've followed all the instructions in Installation.

Download the PR

First, let's download a PR:

$ prr get danobi/prr-test-repo/6
/home/dxu/dev/review/danobi/prr-test-repo/6.prr

prr-get downloads a pull request onto your filesystem into what we call a "review file". On success, the path to the review file is printed to your terminal.

But to be sure, let's check our status:

$ prr status
Handle                  Status     Review file
danobi/prr-test-repo/6  NEW        /home/dxu/dev/review/danobi/prr-test-repo/6.prr

Great! We've confirmed the review was downloaded.

Mark up the review file

Now that the review file is downloaded, let's mark it up. You can open the review file in your favorite editor or use prr-edit to automatically open it in $EDITOR.

$ prr edit danobi/prr-test-repo/6

Your editor should show the contents as something like this:

> diff --git a/ch2.txt b/ch2.txt
> index 4d729e6..2641120 100644
> --- a/ch2.txt
> +++ b/ch2.txt
> @@ -2,13 +2,6 @@ CHAPTER 2. WAGING WAR
>
>  1.  Sun Tzu said: In the operations of war, where there are in the field a thousand swift chariots, as many heavy chariots, and a hundred thousand mail-clad soldiers, with provisions enough to carry them a thousand LI, the expenditure at home and at the front, including entertainment of guests, small items such as glue and paint, and sums spent on chariots and armor, will reach the total of a thousand ounces of silver per day. Such is the cost of raising an army of 100,000 men.
>
> -2. When you engage in actual fighting, if victory is long in coming, then men's weapons will grow dull and their ardor will be damped. If you lay siege to a town, you will exhaust your strength.
> -
> -3. Again, if the campaign is protracted, the resources of the State will not be equal to the strain.
> -
> -4. Now, when your weapons are dulled, your ardor damped, your strength exhausted and your treasure spent, other chieftains will spring up to take advantage of your extremity. Then no man, however wise, will be able to avert the consequences that must ensue.
> -
> -5. Thus, though we have heard of stupid haste in war, cleverness has never been seen associated with long delays.
>
>  6. There is no instance of a country having benefited from prolonged warfare.
>
> @@ -30,6 +23,11 @@ CHAPTER 2. WAGING WAR
>
>  16. Now in order to kill the enemy, our men must be roused to anger; that there may be advantage from defeating the enemy, they must have their rewards.
>
> +asdf
> +asdf
> +asdf
> +adsf
> +
>  17. Therefore in chariot fighting, when ten or more chariots have been taken, those should be rewarded who took the first. Our own flags should be substituted for those of the enemy, and the chariots mingled and used in conjunction with ours. The captured soldiers should be kindly treated and kept.
>
>  18. This is called, using the conquered foe to augment one's own strength.

There's a lot of diff we don't particularly care about. So let's snip away the top portion. We can do this by replacing as many contiguous lines as we want with [...].

After snipping your review file will look exactly like this:

[...]
> @@ -30,6 +23,11 @@ CHAPTER 2. WAGING WAR
>
>  16. Now in order to kill the enemy, our men must be roused to anger; that there may be advantage from defeating the enemy, they must have their rewards.
>
> +asdf
> +asdf
> +asdf
> +adsf
> +
>  17. Therefore in chariot fighting, when ten or more chariots have been taken, those should be rewarded who took the first. Our own flags should be substituted for those of the enemy, and the chariots mingled and used in conjunction with ours. The captured soldiers should be kindly treated and kept.
>
>  18. This is called, using the conquered foe to augment one's own strength.

Looks like someone is trying to ruin a classic text with gibberish! Let's tell them to remove it:

[...]
> @@ -30,6 +23,11 @@ CHAPTER 2. WAGING WAR
>
>  16. Now in order to kill the enemy, our men must be roused to anger; that there may be advantage from defeating the enemy, they must have their rewards.
>

> +asdf
> +asdf
> +asdf
> +adsf
> +

Please remove this text!

[...]

Here we've done two things:

1. Create a "spanned inline" comment that attaches our comment to a block of lines. If we ommitted the leading newline, we would have a regular "inline" comment that attaches to the most immediately quoted line. Using spanned inline comments can be useful when we want to be precise during review.
2. Snip the trailing text. We didn't have to do this, but it makes the example a little clearer.

Submit the review

Now that we're done with our review, it's time to submit it to Github. Do this by running:

$ prr submit danobi/prr-test-repo/6

On success there will not be any output. But just to be safe, let's confirm submission status:

$ prr status
Handle                  Status     Review file
danobi/prr-test-repo/6  SUBMITTED  /home/dxu/dev/review/danobi/prr-test-repo/6.prr

Looks like it made it to the web interface as well:

Homework

Try figuring out how to "request changes" on the PR!

Vim integration

The vim/ directory in prr source contains Vim plugin providing syntax coloring, filetype detection and folding configuration for *.prr files.

To install, modify &runtimepath to add this directory or use a plugin manager of your choice to add this plugin.

Vundle installation

Add the following to your .vimrc:

Plugin 'danobi/prr', {'rtp': 'vim/'}

Manual installation

Copy the provided *.vim files into their appropriate subdirectories in ~/.vim.

Syntax colors

The prr "plugin" exports some preconfigured syntax hooks.

Example from my dotfiles:

"Automatically set up highlighting for `.prr` review files
"Use `:hi` to see the various definitions we kinda abuse here
augroup Prr
  autocmd!
  autocmd BufRead,BufNewFile *.prr set syntax=on

  "Make prr added/deleted highlighting more apparent
  autocmd BufRead,BufNewFile *.prr hi! link prrAdded Function
  autocmd BufRead,BufNewFile *.prr hi! link prrRemoved Keyword
  autocmd BufRead,BufNewFile *.prr hi! link prrFile Special

  "Make file delimiters more apparent
  autocmd BufRead,BufNewFile *.prr hi! link prrHeader Directory

  "Reduce all the noise from color
  autocmd BufRead,BufNewFile *.prr hi! link prrIndex Special
  autocmd BufRead,BufNewFile *.prr hi! link prrChunk Special
  autocmd BufRead,BufNewFile *.prr hi! link prrChunkH Special
  autocmd BufRead,BufNewFile *.prr hi! link prrTagName Special
  autocmd BufRead,BufNewFile *.prr hi! link prrResult Special
augroup END

Folding

With default Vim configuration all folds will be closed by default, so if you want them to be opened then you need to do one of these:

• Add set foldlevel=9999 in your Vim config to open all folds by default
• Add set nofoldenable to disable folding

Consult :h folding for more details.

Configuration

prr supports two types of configuration: global and local.

Global configuration controls global behavior. Local configuration controls behavior for the directory the configuration is in as well as its subdirectories.

prr respects the XDG Base Directory Specification where possible.

Global configuration

prr reads global configuration from both prr --config as well as $XDG_CONFIG_HOME/prr/config.toml. It places priority on the --config flag.

The following global configuration options are supported:

• [prr]
  • token
  • workdir
  • url
  • activate_pr_metadata_experiment

The token field

The required token field is your Github Peronal Authentical Token as a string.

Example:

[prr]
token = "ghp_Kuzzzzzzzzzzzzdonteventryzzzzzzzzzzz"

The workdir field

The optional workdir field takes a path in string form.

Review files and metadata will be placed here. Note ~ and $HOME do not expand. Paths must also be absolute.

If omitted, workdir defaults to $XDG_DATA_HOME/prr.

Example:

[prr]
workdir = "/home/dxu/dev/review"

The url field

The optional url field takes a URL to the Github API in string form.

This is useful for Github Enterprise deployments where the API endpoint is non-standard.

Example:

[prr]
url = "https://github.company.com/api/v3"

The activate_pr_metadata_experiment field

The optional activate_pr_metadata_experiment field determines whether, prr is downloading the PR description as well as the diff of the PR. Note that the effect as well as the name of this option may change in the future.

If this is not explicitly set to "true", it is considered to be set to "false".

Example:

[prr]
activate_pr_metadata_experiment = true

Local configuration

Local config files must be named .prr.toml and will be searched for starting from the current working directory up every parent directory until either the first match or the root directory is hit. Local config files override values in the global config in some cases.

If the [prr] table is provided in a local config file, it must be fully specified and will override the global config file.

The following local configuration options are supported:

• [local]
  • repository
  • workdir

The repository field

The optional repository field takes a string in format of ${ORG}/${REPO}.

If specified, you may omit the ${ORG}/${REPO} from PR string arguments. For example, you may run prr get 6 instead of prr get danobi/prr/6.

Example:

[local]
repository = "danobi/prr"

The local workdir field

The optional workdir field takes a string that represents a path.

The semantics are the same as prr.workdir with the following exception: in contrast to global workdir, relative local workdir paths are interpreted as relative to the local config file.

Example:

[local]
repository = ".prr"

Review file

prr supports various review file markups. This document captures all of them.

Review directives

Description: Meta-directives to give to prr in review comment. Currently only supports approving, requesting changes to, and commenting on a PR.

Syntax: @prr approve, @prr reject, or @prr comment.

Example

Review comment

Description: PR-level review comment. You only get one of these per review.

Syntax: Non-whitespace, non-quoted text at the beginning of the review file.

Example

Description comment

Description: Specialized form of review comment. When activate_pr_metadata_experiment is active, review files come with the PR description quoted at the top of the review file. Comments, when uploaded, will quote the preceding description. PR description will continue to be quoted until a review directive is found.

Syntax: Same as review comment.

Example

Inline comment

Description: Inline comment attached to a specific line of the diff.

Syntax: None-whitespace, non-quoted text on a newline immediately following a quoted non-header part of the diff.

Example

Spanned inline comment

Description: Like an inline comment, except it covers a span of lines.

Syntax: To start a span, insert one or more newlines immediately before a quoted, non-header part of the diff. To terminate a span, insert a inline comment.

Example

File comment

Description: File-level comment.

Syntax: Non-whitespace, non-quoted text immediately following the diff --git header

Example

Snips

Description: Use [...] to replace (ie. snip) contiguous quoted lines.

Syntax: [...] or [..] on its own line. Multiple snips may be used in a review file.

Example

This is a PR-level review comment. The following is a review directive
to approve the PR:

@prr approve

> diff --git a/ch2.txt b/ch2.txt
> index 4d729e6..2641120 100644
> --- a/ch2.txt
> +++ b/ch2.txt
> @@ -2,13 +2,6 @@ CHAPTER 2. WAGING WAR
>  
>  1.  Sun Tzu said: In the operations of war, where there are in the field a thousand swift chariots, as many heavy chariots, and a hundred thousand mail-clad soldiers, with provisions enough to carry them a thousand LI, the expenditure at home and at the front, including entertainment of guests, small items such as glue and paint, and sums spent on chariots and armor, will reach the total of a thousand ounces of silver per day. Such is the cost of raising an army of 100,000 men. 
>  
> -2. When you engage in actual fighting, if victory is long in coming, then men's weapons will grow dull and their ardor will be damped. If you lay siege to a town, you will exhaust your strength. 
> -
> -3. Again, if the campaign is protracted, the resources of the State will not be equal to the strain. 
> -
> -4. Now, when your weapons are dulled, your ardor damped, your strength exhausted and your treasure spent, other chieftains will spring up to take advantage of your extremity. Then no man, however wise, will be able to avert the consequences that must ensue. 
> -
> -5. Thus, though we have heard of stupid haste in war, cleverness has never been seen associated with long delays. 
>  
>  6. There is no instance of a country having benefited from prolonged warfare. 
>  
> @@ -30,6 +23,11 @@ CHAPTER 2. WAGING WAR
>  
>  16. Now in order to kill the enemy, our men must be roused to anger; that there may be advantage from defeating the enemy, they must have their rewards. 
>  
> +asdf
> +asdf
> +asdf
> +adsf
> +
>  17. Therefore in chariot fighting, when ten or more chariots have been taken, those should be rewarded who took the first. Our own flags should be substituted for those of the enemy, and the chariots mingled and used in conjunction with ours. The captured soldiers should be kindly treated and kept. 
>  
>  18. This is called, using the conquered foe to augment one's own strength. 

This is a PR-level review comment. You can only leave one of these per review.

> diff --git a/ch2.txt b/ch2.txt
> index 4d729e6..2641120 100644
> --- a/ch2.txt
> +++ b/ch2.txt
> @@ -2,13 +2,6 @@ CHAPTER 2. WAGING WAR
>  
>  1.  Sun Tzu said: In the operations of war, where there are in the field a thousand swift chariots, as many heavy chariots, and a hundred thousand mail-clad soldiers, with provisions enough to carry them a thousand LI, the expenditure at home and at the front, including entertainment of guests, small items such as glue and paint, and sums spent on chariots and armor, will reach the total of a thousand ounces of silver per day. Such is the cost of raising an army of 100,000 men. 
>  
> -2. When you engage in actual fighting, if victory is long in coming, then men's weapons will grow dull and their ardor will be damped. If you lay siege to a town, you will exhaust your strength. 
> -
> -3. Again, if the campaign is protracted, the resources of the State will not be equal to the strain. 
> -
> -4. Now, when your weapons are dulled, your ardor damped, your strength exhausted and your treasure spent, other chieftains will spring up to take advantage of your extremity. Then no man, however wise, will be able to avert the consequences that must ensue. 
> -
> -5. Thus, though we have heard of stupid haste in war, cleverness has never been seen associated with long delays. 
>  
>  6. There is no instance of a country having benefited from prolonged warfare. 
>  
> @@ -30,6 +23,11 @@ CHAPTER 2. WAGING WAR
>  
>  16. Now in order to kill the enemy, our men must be roused to anger; that there may be advantage from defeating the enemy, they must have their rewards. 
>  
> +asdf
> +asdf
> +asdf
> +adsf
> +
>  17. Therefore in chariot fighting, when ten or more chariots have been taken, those should be rewarded who took the first. Our own flags should be substituted for those of the enemy, and the chariots mingled and used in conjunction with ours. The captured soldiers should be kindly treated and kept. 
>  
>  18. This is called, using the conquered foe to augment one's own strength. 

> First line of description.

The above line will be quoted in Github UI. The comment below will not quote
the PR description b/c the review directive terminates the quoting.

@prr comment

> 
> The second paragraph has three sentences. This is the second sentence.
> The third and final sentence is on a new line.
> 
> Final line.

Final comment; no quote.

> diff --git a/ch3.txt b/ch3.txt
> index 159cf1c..cc376fa 100644
> --- a/ch3.txt
> +++ b/ch3.txt
> @@ -20,6 +20,8 @@ CHAPTER 3. ATTACK BY STRATAGEM
>  
>  10. Hence, though an obstinate fight may be made by a small force, in the end it must be captured by the larger force. 
>  
> +10.5. new line
> +
>  11. Now the general is the bulwark of the State; if the bulwark is complete at all points; the State will be strong; if the bulwark is defective, the State will be weak. 
>  
>  12. There are three ways in which a ruler can bring misfortune upon his army: 

> diff --git a/ch2.txt b/ch2.txt
> index 4d729e6..2641120 100644
> --- a/ch2.txt
> +++ b/ch2.txt
> @@ -2,13 +2,6 @@ CHAPTER 2. WAGING WAR
>  
>  1.  Sun Tzu said: In the operations of war, where there are in the field a thousand swift chariots, as many heavy chariots, and a hundred thousand mail-clad soldiers, with provisions enough to carry them a thousand LI, the expenditure at home and at the front, including entertainment of guests, small items such as glue and paint, and sums spent on chariots and armor, will reach the total of a thousand ounces of silver per day. Such is the cost of raising an army of 100,000 men. 
>  
> -2. When you engage in actual fighting, if victory is long in coming, then men's weapons will grow dull and their ardor will be damped. If you lay siege to a town, you will exhaust your strength. 
> -
> -3. Again, if the campaign is protracted, the resources of the State will not be equal to the strain. 
> -
> -4. Now, when your weapons are dulled, your ardor damped, your strength exhausted and your treasure spent, other chieftains will spring up to take advantage of your extremity. Then no man, however wise, will be able to avert the consequences that must ensue. 

This is an inline comment attached to passage 4.

> -
> -5. Thus, though we have heard of stupid haste in war, cleverness has never been seen associated with long delays. 
>  
>  6. There is no instance of a country having benefited from prolonged warfare. 
>  
> @@ -30,6 +23,11 @@ CHAPTER 2. WAGING WAR
>  
>  16. Now in order to kill the enemy, our men must be roused to anger; that there may be advantage from defeating the enemy, they must have their rewards. 
>  
> +asdf
> +asdf
> +asdf
> +adsf
> +
>  17. Therefore in chariot fighting, when ten or more chariots have been taken, those should be rewarded who took the first. Our own flags should be substituted for those of the enemy, and the chariots mingled and used in conjunction with ours. The captured soldiers should be kindly treated and kept. 
>  
>  18. This is called, using the conquered foe to augment one's own strength. 

> diff --git a/ch2.txt b/ch2.txt
> index 4d729e6..2641120 100644
> --- a/ch2.txt
> +++ b/ch2.txt
> @@ -2,13 +2,6 @@ CHAPTER 2. WAGING WAR
>  
>  1.  Sun Tzu said: In the operations of war, where there are in the field a thousand swift chariots, as many heavy chariots, and a hundred thousand mail-clad soldiers, with provisions enough to carry them a thousand LI, the expenditure at home and at the front, including entertainment of guests, small items such as glue and paint, and sums spent on chariots and armor, will reach the total of a thousand ounces of silver per day. Such is the cost of raising an army of 100,000 men. 
>  

> -2. When you engage in actual fighting, if victory is long in coming, then men's weapons will grow dull and their ardor will be damped. If you lay siege to a town, you will exhaust your strength. 
> -
> -3. Again, if the campaign is protracted, the resources of the State will not be equal to the strain. 
> -
> -4. Now, when your weapons are dulled, your ardor damped, your strength exhausted and your treasure spent, other chieftains will spring up to take advantage of your extremity. Then no man, however wise, will be able to avert the consequences that must ensue. 

This is a spanned inline comment attached to passages 2 through 4.

> -
> -5. Thus, though we have heard of stupid haste in war, cleverness has never been seen associated with long delays. 
>  
>  6. There is no instance of a country having benefited from prolonged warfare. 
>  
> @@ -30,6 +23,11 @@ CHAPTER 2. WAGING WAR
>  
>  16. Now in order to kill the enemy, our men must be roused to anger; that there may be advantage from defeating the enemy, they must have their rewards. 
>  

> +asdf
> +asdf
> +asdf
> +adsf
> +

This is another spanned comment attached to the newly added text, that
GitHub will interpret as a code suggestion [0], suggesting the added text
be removed.

```suggestion
```

[0]: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request

>  17. Therefore in chariot fighting, when ten or more chariots have been taken, those should be rewarded who took the first. Our own flags should be substituted for those of the enemy, and the chariots mingled and used in conjunction with ours. The captured soldiers should be kindly treated and kept. 
>  
>  18. This is called, using the conquered foe to augment one's own strength. 

> diff --git a/ch2.txt b/ch2.txt

This is a file-level comment on ch2.txt

> index 4d729e6..2641120 100644
> --- a/ch2.txt
> +++ b/ch2.txt
> @@ -2,13 +2,6 @@ CHAPTER 2. WAGING WAR
>  
>  1.  Sun Tzu said: In the operations of war, where there are in the field a thousand swift chariots, as many heavy chariots, and a hundred thousand mail-clad soldiers, with provisions enough to carry them a thousand LI, the expenditure at home and at the front, including entertainment of guests, small items such as glue and paint, and sums spent on chariots and armor, will reach the total of a thousand ounces of silver per day. Such is the cost of raising an army of 100,000 men. 
>  
> -2. When you engage in actual fighting, if victory is long in coming, then men's weapons will grow dull and their ardor will be damped. If you lay siege to a town, you will exhaust your strength. 
> -
> -3. Again, if the campaign is protracted, the resources of the State will not be equal to the strain. 
> -
> -4. Now, when your weapons are dulled, your ardor damped, your strength exhausted and your treasure spent, other chieftains will spring up to take advantage of your extremity. Then no man, however wise, will be able to avert the consequences that must ensue. 
> -
> -5. Thus, though we have heard of stupid haste in war, cleverness has never been seen associated with long delays. 
>  
>  6. There is no instance of a country having benefited from prolonged warfare. 
>  
> @@ -30,6 +23,11 @@ CHAPTER 2. WAGING WAR
>  
>  16. Now in order to kill the enemy, our men must be roused to anger; that there may be advantage from defeating the enemy, they must have their rewards. 
>  
> +asdf
> +asdf
> +asdf
> +adsf
> +
>  17. Therefore in chariot fighting, when ten or more chariots have been taken, those should be rewarded who took the first. Our own flags should be substituted for those of the enemy, and the chariots mingled and used in conjunction with ours. The captured soldiers should be kindly treated and kept. 
>  
>  18. This is called, using the conquered foe to augment one's own strength. 

[...]

>  16. Now in order to kill the enemy, our men must be roused to anger; that there may be advantage from defeating the enemy, they must have their rewards. 
>  
> +asdf
> +asdf
> +asdf
> +adsf
> +

We snipped the top and bottom of this document so we can focus on the review comment here.

[...]

Contributing

There isn't much process yet, so this page is light. But a few things to keep in mind:

• I highly encourage floating an idea before implementing it. So that we can avoid any potential wasted work. My (@danobi) SLA for providing some kind of feedback is 1-2 days.

• Tests are mandatory for any parser changes. Tests are currently kinda light especially as it's closer to API call layer, but we should be adding tests where possible.

• All user facing changes must come with documentation update. Documentation lives in book/ and is rendered by mdBook. Documentation changes are automatically deployed upon merge.

Releases

Releases are currently managed by cargo-release.

We are currently pre-1.0, so we are only cutting minor and patch releases. We try to follow semantic versioning. Cut a minor release by running:

cargo release minor

Similarly, a patch release is:

cargo release patch

Github releases and binary artifacts

At some point it'd be good to automatically cut GH releases and upload binary (statically linked) artifacts to it. It's not too hard to do using GHA. Just lazy. Contributions welcome.