oVirt site

Livnat Peer lpeer at redhat.com
Fri Sep 16 05:10:10 UTC 2011


<snip>

>> Voting -
>>
>> 1. If the R-T-C policy is in effect the document says that +1 means
>> ‘I have tested this patch myself, and found it good.’.
>>
>> I find this requirement to be too strict. I think that testing a patch
>> is the responsibility of the patch submitter and reviewing a patch
>> should not require actual testing.
> 
> changed it to reviewed/tested
> 
thanks

> 
>> I suggest the following instead -
>>
>> For me reviewing a patch (+1 for a patch) means:
>>  - The patch make sense
>>  - The patch follows the project standards (e.g. code format)
>>  - The patch takes into account the different aspects/components of the
>> project
>>   - The patch does not changes any core behavior or introduces new
>> concepts that were not discussed in the mailing list beforehand.
> 
> I've incorporated this, take a look and shout if you don't like how I
> did it...

looks good

> 
> 
>> If the reviewer wants to test the patch or thinks additional testing is
>> needed then he better do so but i would not go and set that as a
>> requirement.
>>
>>
>> 2. The document says that a three +1 votes are required for a pass.
>> In the context of patch approval that requirement might be an overkill.
>>
>> I am looking at the engine project as an example there are a lot of
>> patches a day, large part of them are not too complicated and one ack
>> can be enough for validation.
>> Requiring 3 acks can slow the development and i am not sure it is
>> justified/needed.
>>
>> I suggest that by default a patch should require only one ack. If a
>> patch introduces a major change or new concepts the patch submitter
>> should mark the patch as such and ask for 3 acks. In addition if a patch
>> reviewer thinks a patch has some level of complexity he can also ask the
>> patch to have 3 acks before approval.
>>
>> This approach can also contribute for Developing and Maturing projects
>> which do not have the needed resources to deal with 3 acks for each patch.
> 
> 
> I thought I covered this, you want 3 for anything big, but a simple ack
> for minor stuff is fine. I re-read and added a bit to make it clearer.
> 
> 
> 

I see it now, yep.

> All applied to the web site.
> 

Thanks, how about changing the "Adding a sub-project" to "Adding a
project"? i am fine with keeping it sub-project, just wanted to make
sure it did not simply sleep.

Livnat






More information about the Project-planning mailing list