On Thu, Mar 23, 2006 at 12:05:46PM -0500, glyph@divmod.com wrote: [...]

For now, because defining "trivial" is so hard (what if your docstring fix has a typo which actually changes some code which breaks some tests?), and it is always too tempting to classify "just this one" branch as "trivial" because the author wants to merge it really fast and it's not _too_ long, the proposed solution is to have a shared "minor fixes" ticket and branch, which should be reviewed once a week or so, and merged en-masse.

As stated on IRC, I think this adds significant, needless overhead to the process for zero benefit. The risk of a trivial commit (let's say this is defined as purely small cosmetic or documentation changes, and that any code change is considered by definition non-trivial) going wrong is low. Even if it *does* go wrong, the cost of fixing that is low (a simple revert away, and diagnosis is going to be easy with a diff that tiny). The proposed process is out of proportion with the problem. Launchpad requires mandatory reviewer approval for merging to trunk -- with an exception for trivial commits. Trivial commits must have [trivial] in the commit message, or they are automatically rejected. Given we already puts diffs in our commit mails, post hoc reviews are easy if anyone cares. Abusing [trivial] would be dealt with the same as any other abuse. The system works very well for us. I think it would work well for Twisted, and the cost is almost nothing: committers have to add "[trivial]" to commit messages. Vote "no" to mindless bureaucracy! ;) -Andrew.

On 3/23/06, glyph@divmod.com <glyph@divmod.com> wrote:

Right now, there are no exceptions to the rule of "make sure there's a ticket before you check any code in to trunk".

However, several people raised some concerns on IRC over the additional friction this adds for minor fixes (the change in question being spiv's doc correction of "the the").

It's worth noting that if branching is difficult for you, it is also perfectly acceptable to attach a patch to a ticket. The important thing is that a ticket exist first, so that merging and reviewing can be done independently of committing, not that a branch actually be created in SVN.

For now, because defining "trivial" is so hard (what if your docstring fix has a typo which actually changes some code which breaks some tests?), and it is always too tempting to classify "just this one" branch as "trivial" because the author wants to merge it really fast and it's not _too_ long, the proposed solution is to have a shared "minor fixes" ticket and branch, which should be reviewed once a week or so, and merged en-masse.

To be clear: there are still *no exceptions* to this rule. However, please feel free to file a general-purpose ticket for minor fixes and work with as many other developers checking into the branch for that ticket.

Even if we don't allow trivial changes to be committed straight to trunk, I don't really feel good about having a long-lived branch for trivial commits. The ticket would basically be meaningless, I think. Personally, I think what I'll do is let minor changes sit around in my working copy until I decide I want to commit them, at which point I'll make a ticket and branch for it, or just a ticket + patch. (Oh oh, Combinator should grow a feature to create a new ticket and send the current diff as an attachment. :) -- Christopher Armstrong International Man of Twistery http://radix.twistedmatrix.com/ http://twistedmatrix.com/ http://canonical.com/

On Thu, Mar 23, 2006, glyph@divmod.com wrote:

For now, because defining "trivial" is so hard (what if your docstring fix has a typo which actually changes some code which breaks some tests?), and it is always too tempting to classify "just this one" branch as "trivial" because the author wants to merge it really fast and it's not _too_ long, the proposed solution is to have a shared "minor fixes" ticket and branch, which should be reviewed once a week or so, and merged en-masse.

I don't know that I agree in general that we should allow trivial[1] merges without branching or review, but I don't agree with this proposal either, it seems *even more* bolted on to the "file bug, branch, fix, review, merge" model as the "decide it's trivial, fix in place". If nothing else, I've done code review too, and there's quite a lot of headaches involved in reviewing 37 unrelated trivial changes in one go, because they're unrelated and it involves 37 context switches. Why don't we try the stricter model (no commits to trunk that aren't merges, each merge to trunk has its own branch and its own bug) for a few weeks and then we can see: 1. how angry this has made everyone 2. how many changes are backed up in review (This will require some people to keep notes on things like "noticed typo in docstrings, did not fix because did not have 30 minutes" though.) If the answers to 1 and 2 are "very" and "lots"... well, I still don't think the "minor fixes" ticket and branch idea will be a great solution, but we'd have some data to evaluate it on. -Mary [1] Incidently, describing something as 'hard to define' is not an argument against anything. Now that I'm a computational semanticist again, I can tell people (although not on list) in excruciating, gut wrenching detail, just how hard any damn thing is to define, especially when you want bright line definitions as we do here. If you want to try this argument, you need to propose some 'reasonable' definitions first and then explain why they are, in fact, so much suck. Andrew's attempt is reasonable, although I would have thought "non-semantic small changes to documentation" was better than "small changes to documentation." Now... define 'non-semantic' and we're done! ;)