Re: [infinispan-dev] Development process and handling of PRs
On 21/07/2015 13:31, Dan Berindei wrote:
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.
Of course not. But it also doesn't mean that days can go by even for
1-liners.
> - 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
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.
There are high-priority modules (core) and low-priority ones (atomic).
If a test is randomly failing in a low-priority module, it should be
disabled and a Jira created to track its resolution at a later date.
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.
+1
> - 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
All the postponed decisions need the creation of a Jira.
Tristan
--
Tristan Tarrant
Infinispan Lead
JBoss, a division of Red Hat