[Development] [FYI] commit/review policy refactored
oswald.buddenhagen at qt.io
Wed Jul 13 12:48:51 CEST 2016
On Wed, Jul 13, 2016 at 12:15:44PM +0200, Kai Köhne wrote:
> 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.
first-timers can ask in case of doubt. this is meant to be a policy, not
an intro. somewhat unsurprisingly, i finally took the effort to do this
work because i witnessed several unrelated violations in the last weeks/
> > As a Contributor
> Maybe link back to https://wiki.qt.io/Commit_Policy .
it's linked from the reviewer section, which i find more logical.
> "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.
it's the responsibility of both sides. don't pretend that these social
dynamics don't exist.
> 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.
that's already listed in the contribution guidelines and i wanted to
avoid excessive redundancy. i may reconsider, or move parts of the
content. some more linking is necessary anyway.
> > 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.
that's a good idea. a section about staging and re-staging is needed
> > 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?
every approver is a contributor by definition. ;)
but i guess "disregard" is a better word than "override" in this case.
> > 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.
no. it's addressing the contributor.
More information about the Development