zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Camille Fournier <cami...@apache.org>
Subject Re: Process for reviewing submitted patches?
Date Wed, 16 Aug 2017 19:52:49 GMT
A few thoughts:
1) It is impossible for us to set SLAs for ZK patches to be reviewed. If we
were a company making money on ZK and guaranteeing support for customers
who paid us, perhaps we could do that (and for all I know, it's possible
that customers with contracts at various companies that rely on ZK for
their products do get this). I've been watching this project for many years
now, and because there is no one company that is "The ZooKeeper Company",
it's always a hit-or-miss level of participation.
2) Part of the reason it's a hit-or-miss project is that it is, for better
or worse, somewhat complex, and very mission-critical. Especially when we
get new features, determining whether these features make sense for the
operational boundaries of the system is non-trivial. I don't think the
community wants us to rush in patches to just see the project change
(although if you do, please let's hear it).
3) If you want to get your patches committed, you should expect to
follow-up with the group until it happens. This is a community where polite
reminders can be effectively used to cause movement. Again, see: many of us
are truly volunteers. It is also helpful if you make sure that your patches
have tests if at all possible, and generally follow the coding standards.
If you commit something and you get a -1 from reviewbot, actually
addressing that -1 will help. Explaining what you're doing and why helps a
lot. Many of you do this, but it's certainly not something we always see in
every patch.

We're doing better now than we have been in the past, largely thanks to a
lot of attention recently from a subset of the committers (not including me
sorry, I'm writing this email from my vacation which is about the only time
I ever have to focus on the project). Michael had some great comments on
how the community can help, so follow his lead.

Thanks,
C

On Wed, Aug 16, 2017 at 1:26 PM, Michael Han <hanm@cloudera.com> wrote:

> We are using github pull request instead of the old patch approach since
> last October. So the status of JIRA is irrelevant now (in particular, Patch
> Available will not trigger Jenkins pre-commit workflow now.). This was
> discussed on dev list when we moved to github, the thread's name is "[VOTE]
> move Apache Zookeeper to git".
>
> As for how to identify available patches for review, they should be all
> here:
> https://github.com/apache/zookeeper/pulls
>
> To get notified for new incoming pull requests:
> * Watch our github repo: https://github.com/apache/zookeeper
> Or
> * Subscribe to dev mailing list. Because we have git -> JIRA hook any new
> pull request will get cross posted to JIRA which will then be forwarded to
> dev mailing list.
>
> On Wed, Aug 16, 2017 at 10:15 AM, Patrick Hunt <phunt@apache.org> wrote:
>
> > On Wed, Aug 16, 2017 at 9:51 AM, Jordan Zimmerman <
> > jordan@jordanzimmerman.com> wrote:
> >
> > > * Review other people's patch. If you help out, others will be more
> > willing
> > > to do the same for you. If someone is kind enough to review your code,
> > you
> > > should return the favor to for someone else.
> > >
> > >
> > > That's fair - I should personally try to do more of this. I'll make an
> > > effort here.
> > >
> > >
> > It's not clear to me how we are identifying patches for review today. We
> > used to have a very clear process -  a jira needed to be in the "patch
> > available" state in order to be considered for commit.
> >
> > See "contribute" section here, notice that it's watered down from what it
> > used to be:
> > https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute
> >
> > Dan's patch is not in "patch available" state, is that one of the reasons
> > why it's not being moved forward?
> >
> > Patrick
> >
> >
> > > -Jordan
> > >
> >
>
>
>
> --
> Cheers
> Michael.
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message