Issue status update for http://drupal.org/node/28210 Post a follow up: http://drupal.org/project/comments/add/28210 Project: Drupal Version: cvs Component: base system -Category: feature requests +Category: bug reports Priority: normal Assigned to: Anonymous Reported by: robertDouglass Updated by: robertDouglass -Status: active +Status: patch (code needs review) [dirty trick] I'd like to have better ways of making issues like this show up on the devel-list. robertDouglass Previous comments: ------------------------------------------------------------------------ Thu, 04 Aug 2005 17:48:46 +0000 : robertDouglass I've had some fun clicking the "Play patch bingo!" link repeatedly and think it is a very fine addition to the Contributor links block. One thing that really impressed me was the huge number of patches that seem like a good idea, whose authors assert that the patch is ready to commit, and that have several "+1 I reviewed and liked" comments. All of this made me think a little more about the undead issue of whether we need more core committers and how patches can get reviewed and processed in a more organized manner. Then I had an idea. Here it is. In order to 1) motivate the masses of developers to review patches, 2) guarantee that patches don't get buried in the queue and never get read, 3) have a way to *measure* whether Dries and Steven can keep up on their own, why don't we implement a patch review schedule? I imagine it working something like this: * New patches are put into a first-in-first-out queue. This queue is the order in which committers will review the patch. * Committers must (must is a very flexible word here... more about expectations than rules) review the top patch before reviewing the one right in line behind it, kind of like a judge has to review court cases in a fixed order. * The "up next" patches get well advertised so that developers and those interested in a particular patch know when to do their lobbying and when to make sure that things get reviewed by a critical mass of developers so that the committer has enough support to make a decision. * Every patch review must result in a status change or update comment. This is the history or review trail of a patch. This occurs now anyway, so this is no big change but rather an acknowledgment of the expectation that the committers never "silently" look at a patch and then move it to the back of the queue. * Every patch review results in either a committ, having the patch removed from the queue (won't fix), or being sent to the back of the queue with concrete criteria and expectations for when it gets reviewed next. (I'm not suggesting that if Dries writes "remove this space and send a new patch" and someone does this 5 minutes later that the patch has to wait two weeks until it gets to the top of the list) That's the extent of what I propose to add/change. Here's what I hope that we would gain from it. I expect that the testing efforts of the developer community would become more focused, meaning we would be more likely to be testing the same patch at the same time. Why? Let's say that I have a patch in the queue and it is in place 5. That means very soon, Dries or Steven will be reviewing it. My big chance! Since I know that the opportunity is at hand and that the price of not getting it committed this time around will be high (put back to the end of the queue), I will make sure the patch still applies and then get on IRC and say "hey chx, walkah, killes and amazon, please test this patch today... it is up for review and I need some critical mass". The result will be that more eyes and intentions will be focused on the patch at that moment, which hopefully will give Dries the info he needs and has asked for when it comes to review time. Please note that I am not suggesting that committers should be forbidden from following a patch that they're interested in and committing patches from the middle of the queue. I'm proposing a system for addressing those days when they wake up in the morning and say "well, let's see.... maybe I'll review patches today. Let's take a look at that patch queue." The value of the list is that the whole world will know that very soon, the patch on the top of it will be reviewed and when it is, that is the biggest chance for a while that it will get committed. Furthermore, nobody submitting an issue will have the voice in their head saying "hope Dries is online today so he sees this patch before it gets burried in the queue". Instead you'll know that your patch *is* burried at the end of the queue, but that it will definitely, 100% guaranteed, get reviewed eventually. We can also train the masses to participate more in the review process. If the "up to bat" and "on deck" and "in the hole" issues are visible on drupal.org, and if we provide nice instructions for reviewing patches in such a way that people get the idea "oh, this is something I'm expected to do", I think we'll have a higher review rate. Another thing that such a queue would open the door for is an issue freeze that comes before a code freeze in the development cycle (of course critical bugs will always be handled ASAP and not be put in any queue). Maybe two weeks before a code freeze we can decide that all new issues will go into the queue for the next cycle, and the timing of the code freeze will depend on working through the current queue. As it is, there are dozens of small bugfixes in the queue which will likely not see the light of day before the code freeze or release. This method of organizing the queue might help give us more concrete goals for each stage in the cycle. Finally, and this is the part that is hardest to say since I love Dries and Steven and think they do a bang-up job, a system like this would make it very clear if the system isn't keeping up with demand. If we can't even make it through the whole queue once per development cycle, there is a bottleneck that is restraining our progress. Perhaps we'd decide, for example, to not increase the number of committers, but rather that we need a pre-commit team (5-10 people) who are the ones who review every patch, and only the ones they approve get sent to Dries and Steven. A solution like that would make two similar queues; the one I just described with everything in it which will get reviewed in order by memebers of the pre-committ team, and a queue for Dries and Steven that functions the same way but only contains patches that have passed the pre-commit review phase and was promoted by a member of the pre-commit team (perhaps members of the pre-commit team cannot promote their own patches). Ok - there it is. Flame me! cheers, Robert ------------------------------------------------------------------------ Thu, 04 Aug 2005 19:11:09 +0000 : robertDouglass Another benefit that I forgot to mention is that the price of ignoring crappy issues is higher. Take the case where a really important and really anticipated patch is in place 12 and the 11 issues before it are crap, meaning submitted by bad coders, not addressing important issues and generally unlikely to make core. Anyone interested in the good issue in place 12 will be saying "hey cats, can't we clean the litter box here, I mean, what is all this crap keeping us from getting to the real deal". And *whooosh*, 11 crappy issues are marked to "get a life" or at least "won't fix" and removed from the queue. The price of ignoring issues is high already; our good issues get lost in the crap. The real price we pay for this is delayed and comes in the form of a bloated queue that a core committer won't touch with a ten foot clue-bat. By requiring that we clear the crap to get to the meat the price is moved upfront, which might seem more expensive, but the slim and trim patch queue that will result will be worth it and the overall price of crappy issues will be lower.