[drupal-devel] Should we make project issue status more nuanced?
Following up on recent discussions on drupal decision-making and structure (thanks for the many comments, particularly Steven's recent succinct summary, http://drupal.org/node/15916#comment-26377), I'd like to look at some practical improvements to the system we use for evaluating patches, see my reply to Steven's comment at http://drupal.org/node/15916#comment-26627. I know I could suggest this as a patch on the project module, but I'd like to start with a more general testing of the waters. And, if your eyes are glazing over as you mutter "not him again with his structure babble", please bear with me, I think we might be on to something. The basic issue is that too much is put on the single status, "patch". To see the problem in action, we only have to do a search on Drupal project issues with status=patch. I currently get a total of 124 matches, some ranging up to 33 weeks old. I'm not suggesting the CVS review team is doing anything but excellent work. I know that for my part I can't properly keep up even with the issues for the modules I maintain. I have nothing but appreciation for the work Dries, Steven, and Kjartan do. But they're not able on their own to work through everything. When I go through the outstanding patches, I find quite a few that look (to my not wholly qualified eye) ready to apply--that is, they received discussion, support was evinced, issues raised were to all appearances addressed. Some proposals were very small, but useful. Often the only obvious missing factor is that the patch is now outdated, having sat for so long. Others don't meet these criteria (e.g., lack demonstrated support). What happens currently is that an issue jumps straight from "patch" to "applied"--that is, the status change comes *after* the decision-maker (most often, Dries) has made the change. What's missing is a way for Drupal contributors - more specifically, the subset of developers who are qualified to do so - to pre-filter the patches--basically, to put them in a queue for approval, indicating that, in their view, the patch meets our approval criteria (set out in the Handbook at http://drupal.org/node/10262). Concretely, I'm thinking of an additional status category after "patch": * "reviewed" or "tentatively approved" This would be set when a qualified Drupal member (e.g., a maintainer, or other recognized core contributor) judged the patch meets our criteria. To help even further to increase the community role in decision-making (and thereby relieve the pressure or bottleneck on the CVS review team), we could also consider a time factor. I'm thinking that, when a given amount of time has passed after a patch was deemed "tentatively approved" and no further issues have been raised, the patch would (likely, automatically) move to a second additional category: * "pending application" providing a further indication of readiness. Of course, the actual decision would remain with the CVS review team, who could still raise issues (downgrade the status, deny the proposal, etc.). As an additional step, we could create a new drupal.org role, and limit the ability to set these new status categories to users in this role (i.e., recognized core contributors/developers). Thus we could make sure this judgement was made by qualified members (and would be roughly equivalent to what I think we do e.g. for documentation--a role with permissions to edit the Handbook). What to people think of these suggestions? Would they help improve our code review system? Thanks, Nedjo Rogers
The idea of additional statuses (e.g. "reviewed" or "tentatively approved") is good IMHO.
As an additional step, we could create a new drupal.org role, and limit >the ability to set these new status categories to users in this role >i.e., recognized core contributors/developers).
Is likewise a good idea. Ironically, you could get the ball rolling by submitting a patch to the project module. andre Nedjo Rogers wrote:
Following up on recent discussions on drupal decision-making and structure (thanks for the many comments, particularly Steven's recent succinct summary, http://drupal.org/node/15916#comment-26377), I'd like to look at some practical improvements to the system we use for evaluating patches, see my reply to Steven's comment at http://drupal.org/node/15916#comment-26627.
I know I could suggest this as a patch on the project module, but I'd like to start with a more general testing of the waters. And, if your eyes are glazing over as you mutter "not him again with his structure babble", please bear with me, I think we might be on to something.
The basic issue is that too much is put on the single status, "patch". To see the problem in action, we only have to do a search on Drupal project issues with status=patch. I currently get a total of 124 matches, some ranging up to 33 weeks old.
I'm not suggesting the CVS review team is doing anything but excellent work. I know that for my part I can't properly keep up even with the issues for the modules I maintain. I have nothing but appreciation for the work Dries, Steven, and Kjartan do.
But they're not able on their own to work through everything.
When I go through the outstanding patches, I find quite a few that look (to my not wholly qualified eye) ready to apply--that is, they received discussion, support was evinced, issues raised were to all appearances addressed. Some proposals were very small, but useful. Often the only obvious missing factor is that the patch is now outdated, having sat for so long.
Others don't meet these criteria (e.g., lack demonstrated support).
What happens currently is that an issue jumps straight from "patch" to "applied"--that is, the status change comes *after* the decision-maker (most often, Dries) has made the change.
What's missing is a way for Drupal contributors - more specifically, the subset of developers who are qualified to do so - to pre-filter the patches--basically, to put them in a queue for approval, indicating that, in their view, the patch meets our approval criteria (set out in the Handbook at http://drupal.org/node/10262).
Concretely, I'm thinking of an additional status category after "patch":
* "reviewed" or "tentatively approved"
This would be set when a qualified Drupal member (e.g., a maintainer, or other recognized core contributor) judged the patch meets our criteria.
To help even further to increase the community role in decision-making (and thereby relieve the pressure or bottleneck on the CVS review team), we could also consider a time factor. I'm thinking that, when a given amount of time has passed after a patch was deemed "tentatively approved" and no further issues have been raised, the patch would (likely, automatically) move to a second additional category:
* "pending application"
providing a further indication of readiness.
Of course, the actual decision would remain with the CVS review team, who could still raise issues (downgrade the status, deny the proposal, etc.).
As an additional step, we could create a new drupal.org role, and limit the ability to set these new status categories to users in this role (i.e., recognized core contributors/developers). Thus we could make sure this judgement was made by qualified members (and would be roughly equivalent to what I think we do e.g. for documentation--a role with permissions to edit the Handbook).
What to people think of these suggestions? Would they help improve our code review system?
Thanks, Nedjo Rogers
A couple of radical-thought ideas (almost certainly too late for 4.6): * Admin-configurable status options (not all of the existing status fields will work for everybody) Taking that idea a bit further... * Scrap the existing project & issue node types and rebuild them as default packages for flexinode. This would mean flexinode (or a related module) would have to gain a lot of search, sort, and display tabular summaries. IMHO that would be a useful addition. -Eric Andre Molnar wrote:
The idea of additional statuses (e.g. "reviewed" or "tentatively approved") is good IMHO.
As an additional step, we could create a new drupal.org role, and limit >the ability to set these new status categories to users in this role >i.e., recognized core contributors/developers).
Is likewise a good idea.
Ironically, you could get the ball rolling by submitting a patch to the project module.
andre
Nedjo Rogers wrote:
Following up on recent discussions on drupal decision-making and structure (thanks for the many comments, particularly Steven's recent succinct summary, http://drupal.org/node/15916#comment-26377), I'd like to look at some practical improvements to the system we use for evaluating patches, see my reply to Steven's comment at http://drupal.org/node/15916#comment-26627.
I know I could suggest this as a patch on the project module, but I'd like to start with a more general testing of the waters. And, if your eyes are glazing over as you mutter "not him again with his structure babble", please bear with me, I think we might be on to something.
The basic issue is that too much is put on the single status, "patch". To see the problem in action, we only have to do a search on Drupal project issues with status=patch. I currently get a total of 124 matches, some ranging up to 33 weeks old.
I'm not suggesting the CVS review team is doing anything but excellent work. I know that for my part I can't properly keep up even with the issues for the modules I maintain. I have nothing but appreciation for the work Dries, Steven, and Kjartan do.
But they're not able on their own to work through everything.
When I go through the outstanding patches, I find quite a few that look (to my not wholly qualified eye) ready to apply--that is, they received discussion, support was evinced, issues raised were to all appearances addressed. Some proposals were very small, but useful. Often the only obvious missing factor is that the patch is now outdated, having sat for so long.
Others don't meet these criteria (e.g., lack demonstrated support).
What happens currently is that an issue jumps straight from "patch" to "applied"--that is, the status change comes *after* the decision-maker (most often, Dries) has made the change.
What's missing is a way for Drupal contributors - more specifically, the subset of developers who are qualified to do so - to pre-filter the patches--basically, to put them in a queue for approval, indicating that, in their view, the patch meets our approval criteria (set out in the Handbook at http://drupal.org/node/10262).
Concretely, I'm thinking of an additional status category after "patch":
* "reviewed" or "tentatively approved"
This would be set when a qualified Drupal member (e.g., a maintainer, or other recognized core contributor) judged the patch meets our criteria.
To help even further to increase the community role in decision-making (and thereby relieve the pressure or bottleneck on the CVS review team), we could also consider a time factor. I'm thinking that, when a given amount of time has passed after a patch was deemed "tentatively approved" and no further issues have been raised, the patch would (likely, automatically) move to a second additional category:
* "pending application"
providing a further indication of readiness.
Of course, the actual decision would remain with the CVS review team, who could still raise issues (downgrade the status, deny the proposal, etc.).
As an additional step, we could create a new drupal.org role, and limit the ability to set these new status categories to users in this role (i.e., recognized core contributors/developers). Thus we could make sure this judgement was made by qualified members (and would be roughly equivalent to what I think we do e.g. for documentation--a role with permissions to edit the Handbook).
What to people think of these suggestions? Would they help improve our code review system?
Thanks, Nedjo Rogers
Nedjo Rogers wrote:
The basic issue is that too much is put on the single status, "patch". To see the problem in action, we only have to do a search on Drupal project issues with status=patch. I currently get a total of 124 matches, some ranging up to 33 weeks old.
I agree. See my comments below.
What's missing is a way for Drupal contributors - more specifically, the subset of developers who are qualified to do so - to pre-filter the patches--basically, to put them in a queue for approval, indicating that, in their view, the patch meets our approval criteria (set out in the Handbook at http://drupal.org/node/10262).
For reference, here is the bug status tree used by Bugzilla. http://www.bugzilla.org/docs/tip/html/lifecycle.html https://bugzilla.mozilla.org/page.cgi?id=fields.html Note that Drupal's issue system lacks a resolution field and lumps the resolution in with the status value. I think it makes a lot of sense to split these apart. Also note the lack of status values for "unconfirmed" or "verified" issues. I don't think the "patch" status makes much sense because anyone can attach anything and call it a patch but that doesn't make the issue any closer to done. From the perspective of the CVS commiters, using the "patch" status to find issues that can be fixed must not work very well due to the large number of incomplete patches. From the perspective of a developer most issues automatically get the patch status so they'll show up on the mailing list and receive attention. This is the wrong reason to apply any status. A resolved status would rectify this by allowing a developer to confirm that an attached patch resolves the issue and marking the issue as resolved and setting a resolution. The search system should be able to find/filter issues that have an attached "patch" without this status. Cheers, Chris
Chris Cook wrote:
Nedjo Rogers wrote:
The basic issue is that too much is put on the single status, "patch". To see the problem in action, we only have to do a search on Drupal project issues with status=patch. I currently get a total of 124 matches, some ranging up to 33 weeks old.
I agree. See my comments below.
What's missing is a way for Drupal contributors - more specifically, the subset of developers who are qualified to do so - to pre-filter the patches--basically, to put them in a queue for approval, indicating that, in their view, the patch meets our approval criteria (set out in the Handbook at http://drupal.org/node/10262).
For reference, here is the bug status tree used by Bugzilla.
http://www.bugzilla.org/docs/tip/html/lifecycle.html https://bugzilla.mozilla.org/page.cgi?id=fields.html
Note that Drupal's issue system lacks a resolution field and lumps the resolution in with the status value. I think it makes a lot of sense to split these apart. Also note the lack of status values for "unconfirmed" or "verified" issues.
I don't think the "patch" status makes much sense because anyone can attach anything and call it a patch but that doesn't make the issue any closer to done. From the perspective of the CVS commiters, using the "patch" status to find issues that can be fixed must not work very well due to the large number of incomplete patches. From the perspective of a developer most issues automatically get the patch status so they'll show up on the mailing list and receive attention. This is the wrong reason to apply any status.
A resolved status would rectify this by allowing a developer to confirm that an attached patch resolves the issue and marking the issue as resolved and setting a resolution.
The search system should be able to find/filter issues that have an attached "patch" without this status.
Feel free to submit (or send me) patches against the project.module. It's an important tool for our community, so it could do with some more love. -- Dries Buytaert :: http://www.buytaert.net/
On Wed, 26 Jan 2005 12:52:15 +0100, Dries Buytaert <dries@buytaert.net> wrote:
Feel free to submit (or send me) patches against the project.module. It's an important tool for our community, so it could do with some more love.
If the patches I've submitted for project so far receive some love I'll look more kindly upon it ;-)
Chris Cook wrote:
On Wed, 26 Jan 2005 12:52:15 +0100, Dries Buytaert <dries@buytaert.net> wrote:
Feel free to submit (or send me) patches against the project.module. It's an important tool for our community, so it could do with some more love.
If the patches I've submitted for project so far receive some love I'll look more kindly upon it ;-)
I guess I'll have to subscribe to the project module's issues. I'll look at the patches as time permits. Hopefully today or tommorow. -- Dries Buytaert :: http://www.buytaert.net/
As per this discussion, I've produced an initial patch for improving the granularity of issue status, and group-based permissions for specific staus assignments, here: http://drupal.org/node/17052. I'd appreciate review of this to see how well it meets the aim of enabling more skilled developers to review and promote issues towards application.
Nedjo Rogers wrote:
What to people think of these suggestions? Would they help improve our code review system?
Here are two things that could help me: 1. A 'needs work' flag. When a patch can't be accepted as is, it would be nice if we could mark it as such. Furthermore, we could compile a list of work-needing patches, and separate them from the review-needing patches. Right now, some of the work-needing patches remain in the patch queue while others are set to active and thereby removed from the patch queue. Inconsistent, at best. Possible reasons for setting a patch to 'needs work' could be: the patch no longer applies, the coding style needs work, it lacks documentation, one or more things need to be clarified, etc. 2. A 'needs testers' flag. Sometimes I ask more people to test and review a patch. Occasionally these requests go unnoticed. A 'needs testers' flag would be particularly useful for system-wide patches (eg. I'd ask everyone to test the node revisions patch) or in case I'm delegating the testing and decision-making process (eg. I'd consult JonBob about proposed menu system changes). -- Dries Buytaert :: http://www.buytaert.net/
Op woensdag 26 januari 2005 15:30, schreef Dries Buytaert:
[snip]
I would like to add a small feature request here, allthough it might not be the correct thread :) Can we not add a taxonomy-selecter to this? I would like to be able to group projects. For example we work on weblinks as a group, tah twill later on provide a bundle with modules. Another group works on the CCK, thus all CCK issues should be bundled under CCK. That will improve subselecting and thus will give ppl abetter oveview of patches that are useful for them. Regards, Bèr -- [ Bèr Kessels | Drupal services www.webschuur.com ]
participants (7)
-
Andre Molnar -
Bèr Kessels -
Chris Cook -
Chris Cook -
Dries Buytaert -
Eric Scouten -
Nedjo Rogers