← Back to team overview

openerp-community team mailing list archive

Re: pull request reviews

 

Good to know it. Sometimes, I lose the globality of the community ;)

Regards.


2014-08-06 18:24 GMT+02:00 Maxime Chambreuil <
maxime.chambreuil@xxxxxxxxxxxxxxxxxxxx>:

> Pedro,
>
> We are based in Montreal, Canada and could have reviewed it. It's day time
> for us when it's night for you. Like anyone, you don't see much when it's
> late at night ;)
>
> Ping us next time and we could be your third review.
>
> Thanks.
>
> --
> Maxime Chambreuil
> +1 (514) 276-5468 #126
>
> ------------------------------
> That was because the next step I wanted to do (fix PEP8) was inmediately
> after (the same night). Sincerely, I didn't expect nobody review it so
> late, and I need it that because of Travis results for the next PR, that it
> was not going to be tested if we didn't merge that PR. But as I have said,
> that change was not critical, so I merge it with brain.
>
> Regards.
>
>
> 2014-08-06 17:57 GMT+02:00 Maxime Chambreuil <
> maxime.chambreuil@xxxxxxxxxxxxxxxxxxxx>:
>
>> Leonardo,
>>
>> You are right. Anybody can do reviews, but time should be given to do so.
>>
>> That's why we setup some rules to wait for 5 calendar days. The rules
>> does also mention that for extraordinary moments, a pull request can be
>> merged in less than 5 days if approved by 3 reviewers.
>>
>> We are talking about a pull request merged the same day after 2 reviews
>> here and introducing code bypassing the PEP8 compliance check. I am sure
>> even Pedro will agree that it does not allow too much time for others to
>> review.
>>
>> Regards,
>> --
>> Maxime Chambreuil
>> +1 (514) 276-5468 #126
>>
>> ----- Mail original -----
>> Just for the record, I don't object to you reviewing. I should like to
>> encourage you to.
>>
>> Le 2014-08-06 04:07, Lorenzo Battistini a écrit :
>> > On 08/05/2014 09:31 AM, Leonardo Pistone wrote:
>> >> Hi all,
>> >>
>> >> after a discussion with Pedro, Sandy and Maxime on the pull request
>> >> https://github.com/OCA/account-analytic/pull/2 , a general point came
>> >> out: I always thought that anybody can do reviews, and then someone
>> >> from the maintainer team can take the final decision to do the merge.
>> >
>> > I was assuming that too.
>> >
>> >
>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~openerp-community
>> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~openerp-community
>> More help   : https://help.launchpad.net/ListHelp
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~openerp-community
>> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~openerp-community
>> More help   : https://help.launchpad.net/ListHelp
>>
>
>
>

References