Re: Setup non-pushing gerrit instance for glibc.

[Joseph Myers <joseph@codesourcery.com>]

On Thu, 24 Oct 2019, Carlos O'Donell wrote:

> * Try to setup email based code review in gerrit.
>   - Currently email is outbound only.

Observations on teething troubles with the initial setup:

1. What's the status of fixing the problem with insufficient diff context 
in emails when comments relate to particular parts of the diff?  They need 
to quote the relevant amount of context (typically at least the whole diff 
hunk, including the hunk header showing the changed function name) rather 
than just one line and a reference to an external URL.  It's important for 
messages with reviews to be meaningful in themselves without depending on 
external links.  This is a longstanding problem (it was obvious in some 
experiments some people did with proposing GCC patches with Rietveld, 
gerrit's predecessor, several years ago).  Someone in the GDB discussions 
mentioned a prototype patch to add some context to the emails, so maybe we 
could use that patch.

2. Could text comments in emails from gerrit be properly wrapped?  
Messages such as 
<https://sourceware.org/ml/libc-alpha/2019-10/msg00715.html> are hard to 
read in the list archives because of very long lines.  (Of course, diff 
context / quoted source lines should not be wrapped.)

3. Could we document (on the wiki, I suppose) the process for setting up 
git remotes if you want a git repository to get local copies of all 
versions of all changes proposed this way?  My understanding is gerrit 
makes those available as refs named in some particular way, so adding a 
remote with appropriate fetch config should work, but the particular 
recipe ought to be documented.

4. Could we document how to get and keep up to date a complete local copy 
of all the glibc review data in gerrit (comments etc.) using whatever APIs 
are available?  Again, I think the relevant APIs already exist, but how to 
use them for glibc ought to be documented (this is especially the case for 
a service like this that's experimental, and thus might go away in 
future).

5. It would also be useful to have documentation for how someone should 
make a patch series appear appropriately in gerrit if they want to propose 
a series that way.  That means the emails for a patch series should 
include 1/N, 2/N etc. in their subjects (with a 0/N cover letter as 
appropriate).

6. Lower priority, but note there are certain kinds of changes involving 
huge diffs (e.g. to generated files) that thus *would* need a message size 
limit and pointing to a URL for the diffs in that case, for it to be 
possible to handle such changes through gerrit.  (When sending email 
manually for such a change one can always omit the 50 MB of diffs to 
generated files, but as I understand it part of the point of such a patch 
review system is that the system seems the exact change proposed to be 
committed, complete with generated files, so enabling automated testing of 
patch proposals if desired.)

-- 
Joseph S. Myers
joseph@codesourcery.com