oVirt site

[Itamar Heim]

> -----Original Message-----
> From: project-planning-bounces at ovirt.org
[mailto:project-planning-bounces at ovirt.org] On Behalf Of Livnat
> Peer
> Sent: Thursday, September 15, 2011 11:17 AM
> To: project-planning at ovirt.org
> Cc: Carl Trieloff
> Subject: oVirt site
> 
> 
> 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.


I'd extend on both points:
- patch verification is the responsibility of the submitter, but the
actual verification can be done by 3rd party if relevant (user complains
on a bug. patch is sent for user to verify solves the problem. Patch can
be submitted with note of verified by X).
- need to decide if patch must be verified before submitting, or
verification could be done as part of patch review (for example, gerrit
supports verification as part of flow. Some projects may prefer this
approach).
- (using gerrit terminology) anyone can +1 a patch. only maintainer can +2
the patch. a +1 would help a maintainer do the +2.
- going back to my earlier point on committer != maintainer: in a large
project, we'd still like committer to have right to commit to entire git
to make things simple, but may have maintainers (or call them sub
maintainers) per module in the project. A committer would expect (trust
would suffice I hope) the relevant maintainer[1] to give the +2 before
committing the patch[2]

[1] could be multiple maintainers, if patch touches multiple modules
(backend/api/ui/etc.)
[2] maintainer may push themselves, or leave to patch submitter if has
commit rights


> 
> 
> 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
> _______________________________________________
> Project-planning mailing list
> Project-planning at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/project-planning