geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Udo Kohlmeyer <ukohlme...@pivotal.io>
Subject Re: [VOTE] Using Pull Requests over Review Board
Date Mon, 12 Jun 2017 23:20:00 GMT
0 for PR - I feel that both have their benefits and downsides.

  * +1 for PR: As Naba stated, the integration is simplified and using
    Intellij there is even support to create a PR. Single system to rule
    them all.
  * -1 for PR: I fear that given the huge amount of incoming PR's we
    might loose sight/oversight of any PR's that need to be approved
    from external submitters. It is impossible for external submitters
    to know who needs to be mentioned in order to have the PR looked at.
    With the Reviewboard + Apache Geode PR system, those can be kept
    separate and some oversight is possible.


On 6/12/17 11:13, Nabarun Nag wrote:
> +1 On using PR. I feel using github PR is more convenient because of the
> following reasons
>   a. Commit messages are automatically made into the header and description
> in PR, whereas in reviewboard, you upload the patch and do all the writing
> - manually. It feels redundant.
>   b. After changing the code as per some review comments, we need to upload
> the new patch back to  reviewboard but in case of a PR we just need to do a
> git push.
>   c. A single system for review for all types of contributors - committers
> and non committers.
>   d. Travis CI runs on the PR - detects missed out spotlessApply or merge
> issues.
>   e. Also in case someone is AFK for a long time, one can request a fellow
> committer to merge the PR without losing commit credit.
>
>
>
> After some google search, I could see that a lot of projects are moving to
> Github PR system over Review Board. Apache Spark, Apache CloudStack ,
> Apache SystemML are a few examples.
> Regards
> Nabarun
>
> On Mon, Jun 12, 2017 at 9:37 AM Dave Barnes <dbarnes@pivotal.io> wrote:
>
>> +0
>> Proposal as written says "...for changes that would require peer review
>> before committing...".
>> If this means that minor changes (in my case, typo repairs are the common
>> case) can be checked in without going through a PR process, I can go with
>> the group decision.
>> Still see no compelling need to retire the Review Board, which (as pointed
>> out in the discussion thread) has its uses and advantages.
>>
>>
>>
>> On Mon, Jun 12, 2017 at 7:57 AM, Bruce Schuchardt <bschuchardt@pivotal.io>
>> wrote:
>>
>>> -1
>>>
>>> It places an unnecessary burden on committers and git history is the
>>> definitive record of changes to the code so github pr history isn't
>> really
>>> very useful.  Also, Review Board is a much better tool for reviewing code
>>> than a github PR.
>>>
>>>
>>>
>>> On 6/11/17 9:51 AM, Jacob Barrett wrote:
>>>
>>>> After a few days of discussion [1] this thread has quieted down so I
>> would
>>>> like to take it to a vote. The proposal is to modify the workflow of
>>>> committers to match that of non-committers such that committers shall:
>>>>
>>>> * Perform all work in progress on branches in their personal forks on
>>>> GitHub rather than directly on the ASF Geode repositories.
>>>>
>>>> * Submit GitHub pull requests, following all current rules for pull
>>>> requests, for changes that would require peer review before committing
>> to
>>>> the production branches.
>>>>
>>>> * Register their GitHub accounts with the ASF so that committers can be
>>>> identified in the GitHub mirror repositories.
>>>>
>>>>
>>>> Please cast your vote.
>>>>
>>>> [   ] +1  Approve
>>>> [   ] +0  No opinion
>>>> [   ] -1  Disapprove (and reason why)
>>>>
>>>>
>>>> [1] [DISCUSS] Using Pull Requests over Review Board
>>>> <http://geode.markmail.org/message/row5hgz5zw2ooadl?q=list:
>>>> org%2Eapache%2Egeode%2Edev+discuss+pull>
>>>>
>>>> -Jake
>>>>
>>>>


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