This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Setup non-pushing gerrit instance for glibc.


On Thu, 2019-10-24 at 21:41 +0000, Joseph Myers wrote:
> 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).

I haven't spent much time with Gerrit but the time I have spent
I've found myself thinking Gerrit is designed with the assumption
that submissions are "one" patch.

I found handling a patch series (which I almost always have when
doing changes) rather painful!

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]