For clarity: I think Launchpad replacing Trac is a good thing. I realize that's a huge ordeal. However, I don't think the basic ideas are so different that it'd be impossible. As discussed on IRC, the main downside (aka why we can't do it right now) is lack of notifications, so it's hard to integrate stuff like buildbot yet, but that's being worked on.
The idea I'm proposing is probably doable without Launchpad, but it's definitely much harder without bzr. Mixing bzr and svn, might work, but the developers definitely need to be using bzr because branching really can't be a pain for it to work. I have diagrammed the quantum-transmogrifier example that I tried to explain in the last email. http://bit.ly/aA20Qs On Thu, May 27, 2010 at 7:02 AM, Glyph Lefkowitz <gl...@twistedmatrix.com> wrote: > > On May 26, 2010, at 5:41 AM, Laurens Van Houtven wrote: > > So, while I can definitely sympathize with a certain animosity towards > trac, and I can appreciate the goals and sensibilities of launchpad, I > will probably flat-out veto any required / process-driven usage of > Launchpad blueprints. Bugs, features, enhancements, etc, are all units > of work that need to be tracked, and it's better to have one kind of > crummy interface for tracking _everything_ than three interfaces, even > three good interfaces, for tracking little bits stuff in different > ways according to arbitrary distinctions. (As someone recently opined > to me, Blueprints are a giant complicated interface for pasting the > URL to a Google Wave into a text field. We might as well skip the text > field and just link straight to the conversation from a Trac ticket.) (was it dash? ;-)) I understand your point of view, but I don't think blueprints are that bad. I'm not saying blueprints aren't fat pointers to URLs, but I just don't think that would necessarily make them less useful. As far as arbitrary distinctions go: I'd think new features are blueprints, and bugs are bugs. It's not very arbitrary in my mind -- which is just a different way of saying "I can't think of any grey areas". (Yes, this means there are very few blueprints. I think that's a good thing :)) I think I understand the reasoning behind your opinion from a project lead/release management/developer perspective: both bugs and blueprints are jobs that still need to be done, similarly tracked for releases, and they both take developer time to be resolved. I don't think this reasoning is wrong. For both users and developers, I think thinking of bugs and new features as separate things makes sense. Furthermore, Launchpad has stuff like milestones and targeted releases, so I don't think the three-good-interfaces thing is really that prohibitive. Personally, I don't feel that split is bad for developers either. (FWIW: yes, I think Launchpad's Whiteboard feature needs extending and it probably needs a comment system. And once you do that, you might indeed wonder what the difference with bugs still is -- but I'm not arguing Launchpad is perfect, I'm arguing it's better than Trac ;-)) Even if blueprints are non-negotiable, I think most of what I said could just as well be applied to Launchpad bugs: you'd treat Launchpad bugs like you treat Trac tickets now. Merge proposals and the reviews they come with are properties of branches in Launchpad, and not of blueprints or bugs (IIRC). So, feel free to scrap blueprints, it's not that big a deal :) > lp:~lvh... isn't a verb. What do you do with that string? :) Sorry, bad emacs VC mode habit. I meant 'create a branch lp:~lvh/...' > > and it turns out FTL travel is really really hard so I need two > > smaller branches lp:~lvh/twisted/quantum-transmogrification-tunnels > > and lp:~lvh/twisted/quantum-transmogrification-ansible. Both are > > good, so they get put back into > > ~lvh/twisted/quantum-transmogrification-ftl-travel. > > > > Each review would verify that all children (if any) have also been > > reviewed. So, the final review is pretty small, as suggested :-) > > The review wouldn't verify that the parent had been reviewed, though. > If you started this process by writing a bunch of code in the q-t-f-t > branch, *that* code would never have been reviewed; unless q-t-f-t > needs to be reviewed in its entirety before landing on trunk. Which is > precisely what I'm trying to avoid. Yeah, this is sort-of fixed in practice by my point (1) below, but requires some conscious effort and discipline from the developer. An alternative idea to just having merges of reviewed branches in q-t, would be to have the review of q-t be "all of the commits that aren't reviewed merges from other branches". That sounds really, really annoying, so I'd rather do it the first way. Specifically, that means "don't do that, branch early and often, merging is easy but branching halfway is confusing". > > > This does not limit a developer's freedom to branch at will, because > > code review is opt-in (merge proposal), not opt-out. If you don't do > > it, that code in that branch isn't covered by a previous review, and > > must be reviewed later. > > This strikes me as placing a pretty nasty burden on the reviewer. The > reviewer has to figure out if there are any commits that went only to > the integration branch, isolate them, review them, get that branch > into an OK-it's-reviewed state, while meanwhile other developers might > be committing stuff to that branch and changing its contents, both > from regular working commits and from reviewed merges. It sounds like > a nightmare. Maybe bzr makes it easier than it sounds, but it sounds > bad enough that even a big improvement would still be pretty bad ;). Again, I think point (1) addresses this: yes, but not if you promise to make branches off your first branch off trunk (wording is a bit off, but look at the diagram for clarification). That way they only have merges from other branches, and those merges are reviewed. As long as you don't do that, and keep your development out of review branches, there is no problem. That sounds like a very big caveat, but we have found it to work in practice. I'm not sure why, but one explanation would be that people sometimes hugely underestimate how much time something takes to develop, but guesstimates about the complexity of a particular feature tend to be much more accurate. Even if that goes awry, there is quite some leeway here: the complexity of a review branch has to really completely get out of hand before it wouldn't be okay for it to be one code review anymore -- up to the point that it probably wouldn't pass review anymore under the old design. An added bonus is that there is reduced incentive to keep piling on features in a single review branch, because all of it has to be reviewable in one go. I think this is a good idea, because it encourages proper planning and up-front specs of which features you want to implement. This effect might be stronger in a small, tight development team such as in a small development house than with a distributed development team like Twisted (screwing over your reviewer just means he'll be less friendly to you next time you have to do reviews, and you still have to work with these people later on), but I'm going to be optimistic and pretend we're all nice people :-) > > How exactly code review coverage would work is somewhat of an open > > question and it's the obvious failure in this system. We use it in > > production and it turns out to not be a problem, because people > > always end up doing two things: > > Who is "we"? What is "production"? Are you talking about Twisted or a > hypothetical project which uses Twisted, or a fork of Twisted on > Launchpad? Is this a hypothetical project or a real proejct? I am > super confused. This is a real project that uses (amongst other things) Twisted. > > 1) always branch at least once from the first branch off trunk (so > > branch off lp:~lvh/twisted/quantum-transmogrification). Net result: > > lp:~lvh/twisted/quantum-transmogrification only introduces code in > > the form of merges. > > That's pretty much what I'm proposing, except I don't actually care > whether they're merges or patches or individual commits, as long as > they've cycled through code-review properly. Right, but wouldn't it then be hard for reviewers to know what has been reviewed and what hasn't? > > > 2) always do code review on branches being merged into your first > > branch off trunk (so everything merged into > > lp:~lvh/twisted/quantum-transmogrification has to be reviewed > > already) > > And this is what we already do. Huh? I thought it got reviewed when it was put up for review for suggested merging into trunk. My suggestion is the same thing, except s/trunk/q-t/. There's a second review when q-t itself gets merged into trunk, but as long as those are all merges of reviewed branches, that review is trivial. See diagram, points 16 and 17. > > Note that our merges into trunk are automagic. > > (Again, who is "we", and by what mechanism are they automated? Are you > proposing that we do this, or are you stating that some other people > do?) The aforementioned project using Twisted. I'd prefer not going into a lot of detail, maybe we should forget about it for purposes of keeping the discussing focused. Perhaps what _can_ be taken home from this for purposes of the discussion is that this way of organizing branches does actually work for at least one development team somewhere. I'm not sure to what extent this carries over to Twisted. All I know is that the distributedness of Twisted development isn't much of a problem, since I had no issues and I spent 99% working from home/university. > > If it's merged into a direct branch off of trunk and it satisfies > > some qualities (such as full test coverage :)), it gets put into > > trunk, and that gets pushed to production servers. No human > > interaction. Scary at first, but then you realize humans were > > already involved in the QC process extensively at every point -- > > doing it this way just makes them take testing more seriously :) > > Human interaction of some kind should definitely be required for > Twisted. This is not just pushing some new widget to a web site; this > is potentially pushing out new APIs that need to be documented and > supported to a whole ton of developers. The whole point of the process > modification I've proposed is to make sure that features arrive in > releases as coherent, comprehensible whole pieces, not to allow things > we can automatically verify (like docstring and test coverage) to be > deferred to later merges. These properties of the code should still be > verified on every merge to the integration branch; the interesting > thing about the merge to trunk is the verification that the unit is a > coherent whole (and in the case of a deprecation / replacement, that > the replacement is a functionally adequate upgrade). Yeah, I can see that. My point is not so much an argument for implementing automatic merging into trunk for Twisted, but mostly that this method, when properly implemented, results in a very high confidence level of the quality of your implementation branches, up to the point where people have successfully automated it :) As far as the coherent, comprehensible releases, that's one of the reasons I like Launchpad's milestones, series, blueprints... You want to do all of that and I think it's a great idea, and I think that they're good tools for making all of the specifics of that intent (coherent, comprehensible releases) more transparent to the outside world. > Thanks, > > -glyph Thanks, Laurens _______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
