oVirt site

Carl Trieloff cctrieloff at redhat.com
Thu Sep 15 16:10:09 UTC 2011


On 09/15/2011 04:17 AM, Livnat Peer wrote:
> Hi,
>
> I reviewed the documents on the site and it looks great.
>
> Here are some comments i have on the different documents:
> -----------------------------------------------------------------------
>
> Becoming a maintainer -
>
> A very good document.
>
> typos:
> - "(More on joining the board here …)" - add a
> link to the board doc
> - " to some projects that to others " - s/that/than
> - "or to be become a maintainer" - remove the 'be'
>
> -----------------------------------------------------------------------
>
> Adding a sub-project -
>
> - I think the title should be "Adding a project" to be consistent with
> the rest of the docs on the site.
>
> typo:
> - "In addition the board my vote" - s/my/may
> - "theoVirt" - the oVirt
>
> -----------------------------------------------------------------------
>
> 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


> 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...


> 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.




>
>
> typos:
> - "majour" - s/majour/major
> - The title "Vetos" is lost in the Votes on releases section.
>
> -----------------------------------------------------------------------
>
> Board -
>
> typo:
> - "with 10 or more resource" - s/resource/resources
>
> -----------------------------------------------------------------------
>


Thanks,

All applied to the web site.

Carl.




More information about the Project-planning mailing list