[Libreoffice-qa] gerrit [was: minutes of ESC call ...]

[Bjoern Michaelsen]

On Fri, Jul 13, 2012 at 12:21:17AM +0200, Lionel Elie Mamane wrote:
> On Thu, Jul 12, 2012 at 04:55:50PM +0100, Michael Meeks wrote:
> 
> > * gerrit update (David / Bjoern)
> > 	+ can we have the patch in the 'new change' mail (Lionel)
> > 		+ would be nice !
> > 	+ two ways to do mailing - better to move to using stream object
> > 	  and generate our own mails (Bjoern)
> > 		+ change templates on server-side; would affect all
> > 		  subscribing to a watched project.
> 
> This was said in the context of "can we have the patch in the 'new
> change' mail". Frankly, if I subscribe to a watched project, I'd like
> to get the patch in my personal email, too; not only in the dev ML
> mail.
> 
> So personally, we can change the templates server-side, wouldn't
> bother me, quite the contrary.

My concern is rather, that in the long run:

 - personal watches
 - firehosing the mailing list

are rather distinct issues, and we should handle them differently. The firehose
is already tricky enough as it 'hits' a lot of people (each having a different
preference and workflow) with different backgrounds:

 - experienced devs wanting to see a new patch is available
 - newcomers wanting to learn about the project

As said elsewhere, we can expect experienced devs to set watched projects
themselves, so IMHO we should 'optimize' this as follows:

 - the mails to the devlist to be informative for newcomers
 - the project watches should be helpful to keep reviewers in the loop

Since these two goals lead to very different output in the end we should use
different methods to generate them:

 - optimize the 'watched projects' templates for reviewers
 - generate the firehose not by mail mangling, but by watching stream events
   and generating a custom email from that

> I kinda understand where that's coming from, but frankly I find that
> too strict / restrictive from gerrit's part; if the patches commute
> purely on basis of "do not touch the same lines" (one applies cleanly
> without the other), then just make them "independent". Yes, might miss
> "semantic dependencies" like "added a function in a .hxx" in one
> commit and "use that function" in another commit. But in case of
> doubt, err on the side of *not* annoying the user.

This discussion is mostly sophistic.  If the patches are all accepted this
makes no difference at all anyway. If a patch depends on one that is rejected,
there are two scenarios:

- The patches are indeed interdependant
- The patches are not really interdependant

In the first case, there is nothing automagically to be done: A dev needs to
untangle the mess. In the second case the patch will still apply cleanly on
master, so there is no drawback right now.

Once gerrit will 'own' the repo there will be a difference in this case though
as gerrit will not merge that patch automatically: It will wait for either the
one it depends on to be fixed or the patch to be cleanly rebased upon master.
This is a Good Thing(tm) as the patches are assumed to be interdepandant.

However, a reviewer can still simply push that patch manually to master after
checking the change to be Good -- essentially doing what we do right now (as we
still need to push to fd.o).

> This "resets --hard" my local branch. Bleh. My local branch should be
> treated as a "queue" of patches I want to have in this branch, and
> that are not there yet. 

Nobody forces you to use ./logerrit. Its there for the people who dont want to
be bothered with the details of git and its branches and should be as simple
and trivial as possible. Feel free to generate your own tooling doing things in
whatever way you like.

> Don't force me to meddle with my local queue,
> just handle it reasonably:
> 
>  1) Easy way to push just _one_ commit (from the middle of the queue,
>     obviously, else where's the fun) to gerrit.
> 
>  2) Also an easy way to push my whole queue. (That's what gerrit does
>     now when I push my top ref.)
> 
>  3) When I repush my queue, recognise that I didn't change anything
>     (except rebasing with changes that came in since then, thus new
>     commit timestamp) and don't create a new patch set, and don't
>     respam the reviewers.
> 
>     If I changed one or several of the commits, *then*, OK, do the
>     "new patch set, mail reviewers" thing.

I dont force you to do this, but I wont write these scripts for your either.
This is nothing personal, but experience tells me there are 50 ways to skin a
cat and when it comes to tooling -- and developers will use all 100 of them.
logerrit is a simple, minimal example on how to do the most basic things and is
targeted at the newcomer who doesnt want to be bothered with git concepts.

If you want to do fancy things beyond that, go ahead, note that David is already
working on:

 http://wiki.documentfoundation.org/Development/GitReview

to tweak the OpenStack tooling for LibreOffice -- you might join forces.

Best,

Bjoern