-----Original Message-----
From: project-planning-bounces(a)ovirt.org
[mailto:project-planning-bounces@ovirt.org] On Behalf Of Livnat
Peer
Sent: Thursday, September 15, 2011 11:17 AM
To: project-planning(a)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(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/project-planning