Christian Boos wrote: > Eli wrote: >> On Sunday 02 September 2007 4:26:50 pm Eli wrote: >> >>> Seeking to improve software quality and move toward a solid 0.11 release, >>> the following developer policy is proposed: >>> >>> 1 Anything beyond a 'trivial' bugfix requires a branch. >>> 2 A branch must be reviewed and approved by another developer before >>> merging >>> to trunk. If something is committed to trunk with insufficient review, >>> anyone can backout changes, preferably to a branch. >>> 3 Branches should have a purpose statement on t.e.o under TracDev/Branches >>> and an announcement to trac-dev. >>> >>> Votes/Comments? >>>
+1 for me. I'm also +1 with the comments from Alec and Christopher > Sorry Eli, I'm -1 on this proposal, as I don't think that > /systematically/ creating branches for non-trivial bugfixes will be > productive. > Right now, a bug fix already requires you to reproduce the issue, fix > it, check that it works well and that you didn't break anything else as > a side-effect, write a commit log message and close the ticket. If you > add to this the need to create a branch, make an announcement on > trac-dev, wait for feedback ... well, it's simply not realistic. While this does get into the subject definition of "trivial," I think you're missing the point. Many bugs require a 5 to 10 line patch that fixes the bug and doesn't affect other systems. I'd call that "trivial." However, when the bug fix requires changing a vast number of files, and possibly a slight change in internal or external behavior, this is a prime candidate for a branch. > A policy of intermediate strictness would be to have to post a patch on > the corresponding ticket before committing, eventually creating the > ticket first if there's none yet. As a matter of fact, this is already > what I do quite often. You get notified of this activity if you follow > trac-tickets (everybody here *do* follow trac-tickets, right?). I think the proposal of requiring that the patch be posted on a corresponding ticket (possibly requiring the creation of said ticket) is a good suggestion and should be added to the proposal. However, I don't think it's good replacement for working in a branch > Creating a sandbox branch is always possible, but this should be done at > the developer's discretion rather than made mandatory. In my opinion, a > branch is worth creating when it's supposed to handle multiple related > issues at once. In this case, documenting the branch with a wiki page is > indeed a good idea. Another situation when it becomes natural to create > a branch for a specific ticket is when there begins to be too many > patches to make it convenient (see #199). I think this gets back to the definition of "trivial." I think that all new features should first be done in a branch. I also think that branches should be as specific as possible. A branch that tries to do to many things at once is just a harbinger for disaster > In any case, when the fix or feature seems to be good enough, it's often > better to promote it to a shared place as soon as possible, so that it > gets wider exposure among the developers and users. This facilitates > merging among different changes and helps to uncover unexpected > side-effects early. 'trunk' is actually the natural place for that, as > it's the common place where everyone ends up working one day or another. Merging to trunk should only be done after sufficient review from others. Merging to trunk so that others will review the code is bad practice. Obviously, it'll be nigh impossible to discover and fix all issues in the branch, but the feature should be vetted by as many as possible before it gets merged back to trunk. > However, I understand that in the process of finalizing a release, the > above can be seen as being too "risky", as more and more people are > getting serious at using 0.11 and are currently relying on the trunk for > that. But rather than changing our current usage of trunk, we could use > a stabilization branch and merge there the known good state of trunk, a > bit similar to the way you're currently handling the sandbox/testing > branch. Early adopters could continue to use trunk as usual, those > expecting some real stability for 0.11 could start using the > stabilization branch. Trunk should be stable > To summarize, I'm against establishing a policy of branching for 'small' > changes and instead I'd rather see us do the following: > - merge the sandbox/testing branch in trunk, > - create a branches/0.11-stable branch from that, > - go on with trunk as usual, i.e. commit there when we think a change > is good enough and ready for wider exposure, but without being paranoid > about it > - then periodically merge known good state from trunk into 0.11-stable, > probably discussing on trac-dev which revision range we should merge. Of > course, such a revision should pass all the functional and unit-tests. > - once we feel like it, we tag a trac-0.11-pre1 (or b1, rc1 .. > whatever) from 0.11-stable and release it for wider testing > - repeat periodical merge from trunk to 0.11-stable > - same procedure for reaching 0.11 itself (or a second release > candidate if needed) > - at that point, 0.12 stuff can begin to take place on trunk > - from then on, only selectively merge fixes from trunk to 0.11-stable > (i.e. the way we handled 0.10-stable) Trunk should be stable. We shouldn't have high churn in trunk. It causes problems. The churn should be happening in branches. Being able to limit the churn that happens in trunk will also make it easier for plugin devs to update their plugins in a timely manner. Also, having a stable trunk will allow us to make more frequent releases. If there is a feature that we want to be in the "next version" and it is being developed in a branch, and we find out that it's going to take longer than expected, we will still be able to make a release and simply push it off to the next version. We're kind of in this spot right now with regards to the context stuff and security. Security got merged, and I'm glad for that, but it's only finished for the wiki. It needs to be extended to the tickets, and needs to be done before a release. With context, it's in trunk, but now we're talking about doing some more changes (http://groups.google.com/group/trac-dev/browse_thread/thread/f6fb241963be9178/30c7f81b8facd95b?#30c7f81b8facd95b). These are things that should really have been completed in branches before being merged back into trunk. [1] >> 2 'backout to a branch' means reverting a changeset on trunk, creating a >> new >> branch, and committing the reverted changeset to the new branch. While not >> strictly required, I do see that as a more polite way of reverting someone >> else's change. >> > > Something like > http://trac.edgewall.org/log?action=stop_on_copy&rev=5069&stop_rev=5065 ? > > Backing out others change is not a good idea. Even if you have good > intents (as this was the case above), it won't be perceived that way. Well, it seems that this only points to the fact that the Genshi changes should have been done in a branch first, and then merged into trunk. Also, reverting changes should be infrequent, as work should be done on branches and then vetted by other devs before being merged. If that happens, then there shouldn't be any need to revert changes from trunk. With the new policy, if one's changes get reverted, it should be obvious why. This does, however, raise the point that if you find yourself being the one that needs to back out a change due to insufficient review, notification should be given on the dev list with the reason why. -John [1] I say this now because hindsight is 20/20. I was a proponent for the security merge, due to the fact that I really wanted the feature. However, I also have realized that I really want the security on tickets also, and so half an implementation is almost worse than waiting. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Trac Development" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/trac-dev?hl=en -~----------~----~----~----~------~----~------~--~---
