On 23/08/12 05:22, Jeremy Baron wrote:
Hi,
On Thu, Aug 23, 2012 at 2:46 AM, MZMcBride wrote:
Perhaps revisions that receive no feedback for
thirty days should be
auto-merged and auto-deployed.
That's a pretty horrible solution. IMO, all code *must* be reviewed[0]
before merge.
I actually like it.
If "Evil approval bot" mailed you warning that it will merge 12 pending
changesets in two days if there's no action from your part, that would
force some promptly action by you.
I recently had a trivial patch to operations/puppet waiting for more
than a month. When I noticed I hadn't added any Reviewer, and added to
it, the changeset was fixed in the same day. But that also shows that
nobody looked for new changes there.
I have also seen people approving their own commits to core, something
I'm not comfortable with.
We could add some rules like: "If someone with approval to core has only
positive votes, he can approve it after 1 week. If there was no review
at all, he can approve it after 2 weeks".
(Push repositories would obviously still allow auto-approval, and we
could add exceptions for eg. translation changes)
I was also recently unhappy when I discovered that one patch I thought I
had open, had been abandoned without explanation. There can be good
reasons for doing that, this is a bad idea, no longer needed, fixed in a
different way in I123456... or even "closing because it has been waiting
for a new patch for too long and I don't like seeing this open" (which I
suspect was the case), but *please*, if you're closing another people's
patch, leave an explanation!