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!