[Qt-creator] [Development] [FYI] commit/review policy refactored

[Oswald Buddenhagen]

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/
months.

> > 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
anyway.

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

> and should be in the 'As a Reviewer' section. 
> 
no. it's addressing the contributor.