[infinispan-dev] Development process and handling of PRs

Dan Berindei dan.berindei at gmail.com
Tue Jul 21 07:31:25 EDT 2015


On Mon, Jul 20, 2015 at 7:44 PM, Tristan Tarrant <ttarrant at redhat.com> wrote:
> Hi all,
>
> there is something about our current development model which I feel is
> holding us back a little. This is caused by a number of issues:
>
> - Handling Pull Requests: we are really slow at doing this. When issuing
> a PR, a developer expects at least one review to happen within the next
> half-day at most. Instead, requests sit in the queue for days (weeks)
> before they even get considered. I don't expect everybody to just drop
> what they are doing and review immediately, but at least be a bit more
> reactive.

Lately we've had a lot of PRs that take more than 4 hours just to read
and try to remember what was going on in the code that's being
modified. If I issue a PR modifying 50+ files (which I freely admit
I've done in the past) I definitely don't expect the first review pass
to be done within a day.

> - It seems like we're always aiming for the perfect PR. Obviously a PR
> should have zero failures, but we should be a bit more iterative about
> the way we make changes. This is probably also a consequence of the
> above: why should I break up my PR into small chunks, if it takes so
> long to review each one and the cumulative delay is detrimental to my
> progress. I like what Pedro has done for his locking changes.

First of all, I'm not sure our zero failures policy works. I know more
about core, so I try to make sure PRs I review don't introduce random
failures in core, but I'm not sure anyone ever investigated the random
failures in AtomicObjectFactoryTest.

Breaking your work into small chunks is clearly more work, and a small
PR will sometimes take just as long to be integrated as a big one. But
I really think the extra feedback you get by having smaller PRs is
worth it. And if you think your PR is spending too much time in the
queue, it's much easier to ping someone on IRC to review a 50-lines
change than a 5000-lines one.

> - We're afraid of changes, but that's what a development phase is for,
> especially for a new major release. We should be a bit more aggressive
> with trying things out. A PR can be merged even if there are some
> concerns (obviously not from a fundamental design POV), and it can be
> refined in later steps.

I'm ok with postponing some decisions, as long as we're not forgetting
about all the discussions and moving on to the next big thing when the
PR is closed.

>
> This is what I would like to see in Beta2:
> - The functional API (I can take care of rebasing the PR)
> - The management console
> - The query grouping/aggregation stuff
> - anything else we can merge soon
>
> I would like to release Wednesday at the latest, so please do your best
> to help in achieving this goal.
>
> Tristan
> --
> Tristan Tarrant
> Infinispan Lead
> JBoss, a division of Red Hat
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


More information about the infinispan-dev mailing list