
Attached is a message which I wrote to Manuel shortly after ICFP
relating some recent experiences with GitHub's code review
functionality.

Since I wrote this there are a few additional issues that I stumbled
upon write reviewing other patches which I would like to mention,

 * It is not possible to comment on lines that are not in the vicinity of
   a change. I often find myself doing this to point out parts of the
   code that a contributor may have overlooked.

 * I would reiterate the difficulty of keeping ones bearings in a large
   patch. Recently I reviewed Moritz's Relocatable builds patch [1]
   and found myself frequently wondering what file I was looking at.
   While in Differential I can simply cast a glance to the file tree, in
   GitHub I need to scroll up until I spot the beginning of the file,
   and then find where I was previously and resume.

 * I have found that GitHub is quite conservative in pointing out
   similarities between old and new code which tends to obscure what
   should be obvious preserved structure. To take a recent example: see
   GitHub's rendering [2] of a piece of Alan's recent TTG work and try
   to work out what that hunk is doing. Now look at the same hunk on
   Phabricator [3]. The difference is in my opinion far more obvious.

All of these papercuts add up and have a real cost. If the consensus
really is that we want to move review to GitHub then I will certainly go
along, but I do feel that it would be a significant regression.

I have still not yet tried using Reviewable.io, so I can't say with
certainty whether it would address these concerns.

Cheers,

- Ben


[1] https://github.com/snowleopard/hadrian/pull/445/files
[2] https://github.com/ghc/ghc/commit/47ad6578ea460999b53eb4293c3a3b3017a56d65#diff-df7abd11782680fc55c1d1b04747dba9R1103
[3] https://phabricator.haskell.org/rGHC47ad6578ea460999b53eb4293c3a3b3017a56d65#C8856OL84
 

-------------------- Start of forwarded message --------------------
From: Ben Gamari <ben@smart-cactus.org>
To: Manuel Chakravarty <manuel.chakravarty@tweag.io>
Subject: GitHub code review infrastructure
BCC: ben@smart-cactus.org
Date: Thu, 28 Sep 2017 09:20:20 -0400
Message-ID: <87y3ozrmcr.fsf@ben-laptop.smart-cactus.org>

