zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Nixon <brian.nixon...@gmail.com>
Subject Re: [DISCUSS] Voting on pull requests
Date Thu, 06 Jun 2019 17:54:49 GMT
The community should absolutely stand by its bylaws. :)

Those two pull requests (899 and 944) were mine so I'd like to sketch what
I saw as a contributor and hopefully figure out a healthier way forward.
Both were opened in the last two months and got reviewer action within
days. There was a reasonable back and forth between comments and new
commits addressing concerns. Some of the concerns were back by "requested
changes" as is proper. Subsequent commits addressed those comments just the
same as other review comments and this is where I think communication fell
down. I was expecting each reviewer to follow up with some change to their
vote status. In one case, the reviewer continued to interact with the pull
request without clarifying whether more changes were expected. This
generates confusion at a minimum and both pull requests had -1 votes that
had been addressed for three weeks before they were eventually merged.

Ultimately, as a contributor it is our responsibility to push for our
contribution and I could certainly have more aggressively pinged folks on
my pull requests (busy people need helpful reminders sometimes). I'd
recommend though, a bit of followup when using "requested changes" on pull
requests given that it requires three active "approve"s to override one
dangling "requested changes" and there generally aren't four committers
simultaneously interacting with a single pull request.

-Brian



On Thu, Jun 6, 2019 at 9:51 AM Andor Molnár <andor@apache.org> wrote:

> Exactly.
>
>
> On 2019. 06. 06. 14:51, Flavio Junqueira wrote:
> > That's covered in the project bylaws, right?
> >
> > https://zookeeper.apache.org/bylaws.html <
> https://zookeeper.apache.org/bylaws.html>
> >
> > -Flavio
> >
> >> On 6 Jun 2019, at 13:49, Enrico Olivelli <eolivelli@gmail.com> wrote:
> >>
> >> Il gio 6 giu 2019, 12:44 Andor Molnar <andor@apache.org <mailto:
> andor@apache.org>> ha scritto:
> >>
> >>> Hi folks,
> >>>
> >>> I’ve seen 2 patches committed recently with “-1s" from committers on
> it.
> >>>
> >>> https://github.com/apache/zookeeper/pull/899 <
> >>> https://github.com/apache/zookeeper/pull/899>
> >>> https://github.com/apache/zookeeper/pull/944 <
> >>> https://github.com/apache/zookeeper/pull/944>
> >>>
> >>> Not a big deal in this case and I think they were in a good shape and
> >>> ready to commit, but I’d like to clarify how do we handle voting on
> pull
> >>> requests. We use github to prepare patches by creating pull requests.
> >>> Github also has a feature of “reviewing” which means that reviewers
are
> >>> able to “approve”, “comment” and “request for changes”. In terms
of
> voting
> >>> this means:
> >>>
> >>> - “approve” = +1
> >>> - “comment” = 0
> >>> - “request for changes” = -1
> >>>
> >> We should enhance the script (we already did it on Bookkeeper for
> instance)
> >>
> >>> In order to commit a patch we need at least 2 binding +1s without
> binding
> >>> -1. Committers/PMCs are able to veto this way.
> >>>
> >>> Do we agree on this process completely?
> >>>
> >> Sure
> >>
> >>> I know that activity in ZooKeeper community is usually very flaky and
> >>> sometimes it’s hard to find committers to review patches.
> >>
> >> We have a new wave of contributions and new committers, so fortunately
> this
> >> is changing.
> >>
> >>
> >> In these cases we usually just commit smaller patches with a single
> binding
> >>> vote, but I think we should be more careful about binding -1s.
> >>>
> >>> Please in the future if you see my -1 on a patch which you think is
> ready
> >>> to commit, bug me as hard as it takes. I’ll make every effort to
> review as
> >>> soon as possible and apologies for any delay.
> >>>
> >> Sure.
> >>
> >>
> >> Enrico
> >>
> >>
> >>> Thanks,
> >>> Andor
> >
>

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