Re: [Libreoffice] breathing master

On Sat, 2011-05-21 at 14:02 +0200, Bjoern Michaelsen wrote:
> Hi all,
>
> here is a proposal to improve our handling of patching and commits on
> master.
> LibreOffice is aiming for a welcoming and inviting culture to aloow
> newcomer get invaolved fast. This is why we have a very lax requirement
> on commits to master and are inviting people to post patches to the
> dev ML too.
> However, this has shown to be problematic in many ways:
> - Patches and reviews are making the dev-mailing list very noisy.
> - Newcomers are distracted by this.
> - Oldtimers elude the idea on the mailing list being integrative,
>   by filtering out patches (thus having effectively a separate patch
>   mailing list).

I think we have a three things here
a) broken tinderboxes/build
b) overly noisy list
c) unreviewed patches

> - Patches are hard to keep track of in the mailing list, esp. the
>   "needs-how-many-more-reviews" question.

The needs-X-reviews is a bit tedious alright. Maybe these can be bumped
off into a release list or run through bugzilla or sommat instead.

> - Response time to "you broke the master"-emails by tinderboxes are way
>   too low -- the default assumption seems to be: somebody else broke the
>   master.

On this specific topic, the emails from the tinderboxes generally don't
include the actual error where the build broke. That's the no 1 problem
IMO with the tinderbox mails, they don't give the actual error. I'd like
to see build-break mails with the error actually inline in them as a
first step.

This may simply be a outcome of parallel builds ? As a quick-fix maybe
having the buildbots auto restart with a single proc build would resolve
that in that case.

> Here is a proposal on how to solve this:
> - Create an branch named "unreviewed" branching off from master on
>   every Tuesday 00:00 UTC.
> - Any submitted patch is pushed to that branch as is without review from
>   Tuesday until Saturday, the only condition is a proper license.
>   Patches from Sunday and Monday will wait to get pushed on Tuesday.

Is it the case that list-submitted patches are the patches that are
breaking the builds ?. Would be cool to auto commit patches to a branch
and build it and mail results to submitter on failure, but would only
make sense to go to that effort if the bottleneck is a lot of broken
list-submitted patches.

> - On Monday, the commits on the "unreviewed" branch are being
>   collectively reviewed and discussed on IRC.

Hmm, 

> Accepted patches are
>   cherrypicked to master, results are posted to the mailing list, the
>   "unreviewed" branch is deleted. Rejected patches can be fixed an
>   resubmitted.
>
> This would:
> - Prevent patches to get lost in space on the ML.
> - Prevent patches to hang in a "needs one more review" cycle.
> - Limit the waiting time for a patch review to maximum one week.
> - Patch submitters can be around on IRC at review time and clarify
>   questions and receive feedback directly. Otherwise little changes for
>   them.
> - It is easier to identify and find a domain expert when doing a "group
>   review"
> - Reduce the noise on the dev mailing list.

> - Newcomers can be sure that a Sunday evening checkout of master is
>   stable.

I agree that its definitely good policy that the w/e build "just work",
for myself I tend to avoid making dramatic checkings at 5pm on a Friday.
I'm not a massive fan of a collective sit down and patch review,
sometimes I have an hour or 10 mins to do a review or two, and sometimes
I don't. Setting aside a fixed-schedule block of time is problematic.

C.