[Libreoffice] master & patch review process ...

[Michael Meeks]

Hi Bjoern,

On Mon, 2011-05-23 at 14:12 +0200, Bjoern Michaelsen wrote:
> On Mon, 23 May 2011 12:00:08 +0100
> > 	Also, my experience of building master is that it is not
.
> > Of the dozen times I've pulled and updated
> > recently - my build has failed perhaps 5 times, of which 3+ have been
> > simple incremental build failures (fixed by gnumake), and another ~2
> > perhaps real issues, one of which has been fixed by the next './g
> > pull' and ... is there really a problem here ?
>
> Every sixth build breaking (ignoring the incrementals) is not welcoming
> to newcomers, because they cannot get started.

	In my view, the incremental build breakage is more prevalent and thus
worth fixing first before we get too excited about more process.

>  And it is more than every sixth build having serious trouble, master
> was broken over more than the complete last weekend

	Which is bad; no-one denies that - *but* it also seems like you just
saw it was broken, and left it - rather than fixing it: it also sounds
like the problem was extremely trivial - some printf's left in some
module (right?).

> Reviewers have absolutely nothing to do with this

	I was addressing two topics - a) master not building and b) the need
for complicated review processes in the same mail; somehow these were
together in the original thread.

> And unfortunately it goes also in untested -- that is: not even tested
> on _one_ platform.

	So that is bad; but - expected too. Not every corner can be tested, and
from time to time there is breakage. If we are paranoid about this -
possibly we want to suggest our new developers build from the release /
stable branch which is immensely more reliable.

> For testing on your own platform we are equally guilty: When master
> does not build or survive basic testing (as it does often), there is no
> way to test your changes.

	Nah - fix the build; or badger the person responsible, and/or revert
the problem commits. It is good to badger those responsible of course,
helps provide useful feedback.

> As for the "experienced hackers" hunting down the breakers: True,
> that is possible, but a terrible waste of resources.

	Where by 'waste' we mean learning lots of other areas of the codebase,
and solving a problem for people ?

> As is, we have no coordination on who heals master, when it breaks:
> Either nobody does -- leading to frustration everywhere, or multiple
> people do -- thereby wasting resources.

	True; but it should be a priority - right ? People should feel
empowered to commit / revert what you need to to fix things ! :-)

> This is why we should prevent this from even happening as much as
> possible.

	Surely, not as much as is possible; surely in a balanced way when
considered against the risks of making it too difficult and painful to
contribute effectively (cf. OO.o process) ;-)

>  Honestly, as I saw master being basically broken again by a
> somewhat larger push, I was tempted to revert the whole bunch to the
> last known good state.

	Fine - why not ? :-) will be easier with Nobert's single git repo.

> The cheapest fix is not letting stuff break on master. The second
> cheapest fix is letting the guy fix it who broke it ASAP. The most
> expansive way to fix master is to let the most experienced hackers hunt
> the issue in larger sets of commits, while everyone else is standing on
> the sidelines and is getting frustrated.

	Heh - I feel your frustration. But I also know that -the-vast- majority
of commits are small, localised changes that require no big,
heavy-lifting process, and just get strangled by that. Sometimes they
break things - true, but lets deal with that in a way that doesn't harm
everyone else all the time ;-)

	As a personal tip (for someone who also breaks the build sometimes) - I
always run 'git diff' before 'git commit' and read the output - I find
that helps.

	Finally, it is worth noticing that master is of particularly bad build
quality (ie. sometimes it breaks over the weekend) when the majority of
the developers are working on building -3-4 and or -3-4-0 in order to
get a great release :-)

	ATB,

		Michael.

-- 
 michael.meeks at novell.com  <><, Pseudo Engineer, itinerant idiot