oVirt site

Livnat Peer lpeer at redhat.com
Thu Sep 15 08:17:03 UTC 2011


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.

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.

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.


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, Livnat



More information about the Project-planning mailing list