develooper Front page | perl.perl5.porters | Postings from August 2013

Re: the GitHub perl mirror

Thread Previous | Thread Next
From:
Nicholas Clark
Date:
August 19, 2013 10:26
Subject:
Re: the GitHub perl mirror
Message ID:
20130819102604.GK66035@plum.flirble.org
On Mon, Aug 19, 2013 at 07:54:11AM +0200, demerphq wrote:
> On 18 August 2013 03:25, Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote:
> > * Karen Etheridge <perl@froods.org> [2013-08-17T20:42:32]
> >> Would this be acceptable?
> >
> > Right now, no.  The is being done to let us catch bug reports coming from
> > submitters who are confused, not to change our intended workflow.
> 
> I think we should consider changing workflow. The pull mechanism is a
> lot easier to manage than what we have, and it encourages everything
> we want, second person code review, log of patches pending, etc.

The current implementation of pull requests sucks massively because it
generates trivial merge commits, when it could avoid this.

It also encourages merging without running tests.

My understanding was that both git and the Linux kernel implement code
review quite successfully by mailing patches to the dev list. I think that
Ricardo has been keeping his eye open for code review tools (but he'll have
to confirm this or correct me when he's back.)

I feel strongly that simply changing to a "push button merge" system is
*not* going to improve the quality of code that gets merged. It's simply
going to mean that pending patches are trivially applied without review, and
consequentially bugs and issues are going to be added, but only found much
later.

I don't think that it's really a toolchain problem that prevents us from
having code review. It's a combination of a lack of an existing culture of
doing this, a *massive* time constraint, and that the code is spread so
thinly that the chance of finding a second person already familiar with the
code is low.

A particular problem is that for any proposed change, pretty much *everyone*
thinks that someone else knows the code better than they do, so assumes that
someone else can review it more efficiently than them. It's almost always
not true. The result is that to properly review a change, often the reviewer
needs to learn the code from scratch, which takes about as much time as the
original author took to write the change. If the original author goes away,
having scratched their itch, and never submits a second patch, it consumes
about as much core developer time *reviewing* the patch as it would have
done to write it in the first place. Implication - for that case we don't
actually save any time by having third-party contributions. I don't *like*
that conclusion. It feels wrong, and it doesn't scale. But I have trouble
finding holes in it, *given* the specific concern here that we have very
little deep knowledge of our codebase.



Also, code review isn't fun. That needs to be solved, else it's not going to
happen on a volunteer driven project, because no-one will volunteer to do it.


Nicholas Clark

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About