I started out by posting all my checkins as patches. I got few comments/reviews which, in the interest of progress, I took as approval.
Here is my suggestion. We need to have folks claim modules and that means promise to review patches in a timely manner (say a week). Having a procedure of making a jira issue and then having a patch in jira doesn't change the essential issue we have to have folks willing to put out the effort to comment on the issue and review the patches otherwise it is just overhead without any benefit. How about this: I will review all changes to the iocore modules: eventsystem/net/aio/cache/cluster/hostdb/dns/libinktomi++, and, so long as I am in town, I will do it within a week. I need some other folks to do these as well so that they can review my changes. I suggest that we have a list somewhere (wiki?) where these the module owners are listed. We can add a "reviewer" to a checkin and include "unavailable" if there is no review within a week which might encourage someone to volunteer. This is just part of the growing process for the project. We need to get a good eco system going which means modules have to find folks willing to take responsibility for them. Also, there is a good deal of cleanup and reorganization needed which is going to require more timely review of far reaching changes in the short term than when the codebase is better modularized. The alternative is short term instability. One way of dealing with that is regressions and unit tests which is one of the things I am working on in the cache code. I am fine with pre or post review before lockdown of the modules I will review because I will be reviewing all the changes eventually. For large changes I would prefer that they be done on a branch so that I can follow the development and make comments as the work is progressing. It was my thought that this is what the dev branch would be about. I am a bit concerned because it seems like what Leif is saying is that nobody is reviewing the changes. That was not the intent. Quite the opposite in fact. Branches should be a safe place to make checkins to encourage pro-active comment and permit reworking off the trunk. Comments? john On 1/30/2010 8:54 PM, Leif Hedstrom wrote: > Hi all, > > I'm a little worried about the lack of checkin and review process on > the dev branch, that we semi-agreed on for TS development. This is > partially my fault, I initially assumed the dev branch was going to be > for John's cache work, but it got turned into a kitchen sink for lots of > development. As such, I'm concerned how we are going to merge this back > to trunk. I guess it's up to the people working on the dev branch, but > I see three possible alternatives: > > 1) Review of bugs / fixes going into dev branch as they are prepared, > before checkins (we'd have to retrofit a few commits that have already > happened). > > 2) Do a complete review of the dev branch before we merge to trunk, as > one big "review". This would be massive undertaking... > > 3) Owners of code on trunk reopens bugs, with patches, and requests > review for migration to trunk. I.e. single bugs / commits are migrated > to trunk as requested by the people who have committed to dev branch. > > > Please vote / pick one :). My +1 is for #1. In the future, I hope we can > avoid a "dev" branch entirely, and only use branches for single, large > feature rewrites (e.g. one branch for rewriting the HTTP SM). We'd > still, of course, create release branches (e.g. release_2_0, > release_2_1, etc). > > Also, remember the trunk is in no way closed, yet. Don't assume that all > fixes has to go into dev, again, it was primarily created to avoid a > massive and potentially disruptive change to land on trunk before the > 2.0 release. I think several of the bugs committed to dev branch should > have gone through reviews and checkins straight to the trunk. > > I'm going to work on finalizing a "roadmap" for getting 2.0 for Q1, > based on our Hackathon discussions. I hope to have that out early next > week, together with bug lists etc. > > Cheers, > > -- Leif
