[Development] [FYI] commit/review policy refactored

[Kai Koehne]

> -----Original Message-----
> From: Development [mailto:development-bounces+kai.koehne=qt.io at qt-
> project.org] On Behalf Of Oswald Buddenhagen
> Sent: Wednesday, July 13, 2016 11:08 AM
> To: development at qt-project.org; qt-creator at qt-project.org
> Subject: [Development] [FYI] commit/review policy refactored
> 
> just a heads-up that i (finally) split off the review policy from the commit
> policy. see https://wiki.qt.io/Review_Policy .
> if you have suggestions for improvement, please discuss them upfront on irc.

I prefer to reply here :)

The page is obviously written from the viewpoint of a maintainer. I'd prefer to write
It in a less intimidating way to the contributor, and make it helpful for first time,
inexperienced contributors.



> As a Contributor

Maybe link back to https://wiki.qt.io/Commit_Policy .

"Make sure that your commit matches the Qt Commit Policy.

> 1. Invite relevant reviewers.
> * Always invite the respective domain experts, not somebody convenient.

Scrap the 'not somebody convenient'. It's the job of the reviewer to decide what he can approve.

Instead, mention how one can find the 'domain expert'. Something like

 * Domain experts can usually be found by inspecting the git log, and mailing lists. If in doubt also add the [https://wiki.qt.io/Maintainers Maintainer] of an area if there is one.

> 2. Give reviewers ample time to respond.
> * Unless everyone who can be reasonably expected to have a relevant opinion to offer has already done so, a full working day waiting time is the absolute minimum; three days are reasonable.
> * In particular, give watchers (usually higher-level maintainers) enough time to voice concerns even if you did not explicitly invite them.

The sub-points are only valid if you have a +2 already. So maybe move them down to a section ("If your change got approved"). Rather mention that it can take some working days until added people respond.

> 3. Discuss objections. Do not override a -1 unless there is a broad expert consensus that the objection is unfounded.

A Contributor cannot usually 'override' a -1 - that can only be done by an approver. Maybe your point is though that, if you got a +2, and somebody voiced objections, one shouldn't stage it?

> 4. Do not ignore/fight the Early Warning System. Justify each override.
> [...]

Isn't that limited to Approvers, too?

> 5. Do not approve your own changes.
[...]

Again that's limited to Approvers, and should be in the 'As a Reviewer' section. 


Regards

Kai