On Thu, May 31, 2018, 15:08 Eugene Kirpichov wrote:
>
>
> On Thu, May 31, 2018 at 2:56 PM Ismaël Mejía wrote:
>
>> If I understood correctly what is proposed is:
>>
>> - Committers to be able to have their PRs reviewed by non-committers
>> and be able to self-merge.
>> - For non-committers
On Thu, May 31, 2018 at 2:56 PM Ismaël Mejía wrote:
> If I understood correctly what is proposed is:
>
> - Committers to be able to have their PRs reviewed by non-committers
> and be able to self-merge.
> - For non-committers nothing changes.
>
I think it is being proposed that a non-committer
If I understood correctly what is proposed is:
- Committers to be able to have their PRs reviewed by non-committers
and be able to self-merge.
- For non-committers nothing changes.
This enables a committer (wearing contributor head) to merge their own
changes without committer approval, so we
Agreed with all said above - as I understand it, we have consensus on the
following:
Whether you're a committer or not:
- Find somebody who's familiar with the code and ask them to review. Use
your best judgment in whose review would give you good confidence that your
code is actually good.
(it's
Seems like enough consensus, and that this is a policy thing that should
have an official vote.
On Thu, May 31, 2018 at 12:01 PM Robert Bradshaw
wrote:
> +1, this is what I was going to propose.
>
> Code review serves two related, but distinct purposes. The first is just
> getting a second set
+1, this is what I was going to propose.
Code review serves two related, but distinct purposes. The first is just
getting a second set of eyes on the code to improve quality (call this the
LGTM). This can be done by anyone. The second is vetting whether this
contribution, in its current form,
Agree, it sounds good to me.
That's basically what I proposed to the Euphoria DSL team ;)
Regards
JB
On 31/05/2018 20:35, Pablo Estrada wrote:
> In that case, does it make sense to say:
>
> - A code review by a committer is enough to merge.
> - Committers can have their PRs reviewed by
In that case, does it make sense to say:
- A code review by a committer is enough to merge.
- Committers can have their PRs reviewed by non-committers that are
familiar with the code
- Non-committers may have their code reviewed by non-committers, but should
have a committer do a lightweight
In that case, the contributor should be a committer pretty fast.
I would prefer to keep at least a final validation from a committer to
guarantee the consistency of the project and anyway, only committer role
can merge a PR.
However, I fully agree that the most important is the Beam community. I
+1 for the idea of reducing load on committers by involving contributors to
perform detailed reviews. I think this has been the case in practice at
least in some cases. I agree with Thomas Weise that proper long term
solution will be growing the committer base by helping existing regular
If someone is trusted enough to review a committers code shouldn't they
also be trusted enough to review another contributors code? As a
non-committer I would get much quicker reviews if I could have other
non-committers do the review, then get a committer who trusts us to merge.
Andrew
On Thu,
+1
On Thu, May 31, 2018 at 8:55 AM Thomas Weise wrote:
> +1 to the goal of increasing review bandwidth
>
> In addition to the proposed reviewer requirement change, perhaps there are
> other ways to contribute towards that goal as well?
>
> The discussion so far has focused on how more work can
+1 to the goal of increasing review bandwidth
In addition to the proposed reviewer requirement change, perhaps there are
other ways to contribute towards that goal as well?
The discussion so far has focused on how more work can get done with the
same pool of committers or how committers can get
Le jeudi 31 mai 2018 à 06:17 -0700, Robert Burke a écrit :
+1 I also thought this was the norm.
My read of the committer/contributor guide was that a committer couldn't
unilaterally merge their own code (approval/LGTM needs to come from someone
familiar with the component), rather than every
+1
This will help reviews go faster. And in the IO reviews makes extra
sense, because a common need is to ping external people who are not
committers but experts in the respective data stores. Of course this
puts more trust in the committers but makes sense.
On Thu, May 31, 2018 at 3:46 PM
@JB: Yea, just talking about Beam practices, not the ASF rules which allow
a project to choose.
@Robert & Udi: This is explicitly _not_ the norm. It hasn't really changed
since the beginning of the project. Here's the relevant section:
By the way +1
Two reviews is overkill. The review period is already pretty long, so it would
better to increase it more ;)
Regards
JB
Le 31 mai 2018 à 15:34, à 15:34, "Jean-Baptiste Onofré" a
écrit:
>That's not fully correct. A committer can directly commit, all depends
>of the approach used
That's not fully correct. A committer can directly commit, all depends of the
approach used in the project: commit and review or review and commit.
In Beam we decided to do review and commit. So a committer or a PMC or a
contributor should create a PR.
Other Apache projects allow to directly
+1 I also thought this was the norm.
My read of the committer/contributor guide was that a committer couldn't
unilaterally merge their own code (approval/LGTM needs to come from
someone familiar with the component), rather than every review needs two
committers. I don't recall a requirement
I thought this was the norm already? I have been the sole reviewer a few
PRs by committers and I'm only a contributor.
+1
On Wed, May 30, 2018 at 2:13 PM Kenneth Knowles wrote:
> ++1
>
> This is good reasoning. If you trust someone with the committer
> responsibilities [1] you should trust
++1
This is good reasoning. If you trust someone with the committer
responsibilities [1] you should trust them to find an appropriate reviewer.
Also:
- adds a new way for non-committers and committers to bond
- makes committers seem less like gatekeepers because it goes both ways
- might
21 matches
Mail list logo
