I'd like to propose that the following statuses be our default workflow: UNCONFIRMED CONFIRMED INPROGRESS (or "IN PROGRESS" or "IN_PROGRESS") RESOLVED VERIFIED My rationale for this is here: http://groups.google.com/group/mozilla.dev.planning/msg/2e79a56064b7328b Upgraded installations would still retain the old default workflow, for compatibility purposes. However, we should eliminate the magic CLOSED -> REOPENED/UNCONFIRMED transition, I suspect.

Seconded.

i like this, however for sites that don't use the UNCONFIRMED status, CONFIRMED doesn't really fit. aside from NEW, i can't think of any replacement that sounds good thou. PENDING? meh.

s/CONFIRMED/OPEN/ ?

yeah, i like OPEN in place of CONFIRMED

I think it might be good to wait for the dust surrounding the Mozilla discussion to die down before deciding what we do for upstream. There may be some good points and ideas which come up in that thread that we could take advantage of. What migration issues might this change raise? Any? Gerv

Our standard current workflow at Red Hat is the following: NEW | ASSIGNED | MODIFIED | ON_QA | VERIFIED | RELEASE_PENDING | CLOSED We do not use UNCONFIRMED or CONFIRMED. We use NEW to distinguish bugs that have not been looked at. Bugs in progress are ASSIGNED. We use MODIFIED to show that some change has been made but that doesn't always mean a patch is attached to the bug so we can't just look for that one. This is the most used. We have other departments who have their own defined workflow. As long as we can reconfigure the system to get back to this workflow then whatever upstream adopts is fine. Dave

I wouldn't use the workflow used by RedHat. I never understood what MODIFIED is, nor how other statuses fit together (ON_QA is nothing more than RESOLVED, and when you do no QA, you are lost with this status). I like mkanat's suggestion much more, with s/CONFIRMED/OPEN/ and s/INPROGRESS/IN PROGRESS/. And of course, you can customize status names and workflow since Bugzilla 3.2, so no need to worry about it.

Just to be clear, I was not advocating usage of our workflow for upstream. Just stating how we currently define our workflow internally as an example. One of the pluses of recent Bugzilla code is that the workflow is flexible for all and I was merely asking that it remain that way after the changes. Although one thing we would like to see is the ability to define workflow per product since the Fedora community for example uses a different workflow than the RHEL developers but we have worked around that for now. One thing we do enforce is that a user can only go from CLOSED to ASSIGNED when reopening a bug. But then they can go whereever from there. Dave

I would like to again express my support for this workflow (as proposed by mconner and gerv): UNCONFIRMED CONFIRMED READYTOFIX INPROGRESS RESOLVED VERIFIED But I agree that a conclusion should be reached on mozilla.dev.planning before this bug is even thought about.

> But I agree that a conclusion should be reached on mozilla.dev.planning before > this bug is even thought about. as this bug is about the //default// status workflow, any decision falling out of m.d.p has no //direct// baring on this bug. that said, assuming they can come to a decision with in reasonable period of time, the outcome of discussion would be interesting.

I might actually be OK with leaving CONFIRMED as the basic status that a bug is filed into. It would probably encourage people to use UNCONFIRMED more frequently than they do now, because they could even use it to mean, "Nobody has looked at this yet," (which is, in a way, really what UNCONFIRMED does mean). Per-product workflow would also allow us to get rid of the "specialness" of UNCONFIRMED, which would make this pattern even better. (Per-product workflow is another bug, though--but note that since we do have all the magic for UNCONFIRMED, we already have all the code structure in place for this.)

The discussion in the newsgroup seems to be progressing well. I don't think OPEN works, because implies that the other states are not open. There are a set of states Bugzilla defines as "open states" as opposed to "closed states", and this new state is just one of them. So calling it OPEN would confuse people. I'm not sure that CONFIRMED is a good name in installations which don't use UNCONFIRMED. But I'm not sure what a better one would be, for the state whose chief feature is that it's not any of the others, if you see what I mean :-) Max: could you be persuaded to include READY_TO_FIX in the default workflow? Or do you think that extra step is only needed for bigger Bugzillas? (Let's add underscores while we are changing all this stuff. No reason to reduce readability unnecessarily.) Gerv

> (Let's add underscores while we are changing all this stuff. No reason to > reduce readability unnecessarily.) any reason why we can't use spaces instead of underscores? "READY TO FIX" seems redundant to me.

I think Bugzilla itself probably supports spaces, although that makes it not a single mental "token" when writing it, and it would make URLs be READY%20TO%FIX rather than READY_TO_FIX. READY_TO_FIX was chosen over READY because people objected that it could be "ready for QA", "ready for checkin" or any other forms of readiness. Gerv

Personally, I don't think READY_TO_FIX is useful by default. As everyone is free to add new statuses if they want to, they should feel free to do it themselves. But in most cases, a bug which is CONFIRMED is already READY TO FIX. So please don't include it in the default workflow.

(In reply to comment #12) > Max: could you be persuaded to include READY_TO_FIX in the default workflow? Or > do you think that extra step is only needed for bigger Bugzillas? I think that the extra step is only needed for bigger Bugzillas.

Okay, I'm pretty much ready to do this, using the workflow proposed in comment 0, which I think the consensus pretty much agrees on.

Here's what I have so far. This is the basics for implementing the new workflow, but there are tons of places that old behaviors have to be eliminated.

Comment on attachment 398087 [details] [diff] [review] Work In Progress Hey Frederic, could you comment on this before I continue further? I split out the init_workflow stuff into a new location because the basic work needed to do the workflow initialization is very simple, and the work needed to upgrade the workflow is very complex. I also don't think we need to do the duplicate_or_move stuff every time we run checksetup.pl (because Bugzilla is supposed to do that for us), but if you think we do, it would probably belong in init_workflow.

I still think that CONFIRMED is a bad name when standing alone, particularly if using UNCONFIRMED is not the default for new installations of Bugzilla (which I believe it is not). I'm not convinced by Max's logic in comment #11, particularly if using UNCONFIRMED is not the Bugzilla default. Why are we in one sense pushing people towards using a particular workflow (because we think it's "better") but yet we don't make that workflow the default? Other than that, this is all fine with me. Gerv

Hmm. Probably the solution there is to enable UNCONFIRMED by default.

That would certainly alleviate the problem. Have we yet disentangled UNCONFIRMED from the voting system? Gerv

(In reply to comment #22) > Have we yet disentangled UNCONFIRMED from the voting system? It's being worked on: bug 162060

Before reviewing this patch, I would like to have UNCONFIRMED be treated as a "normal" bug status, i.e. being available by default. So I suppose this means bug 162060 must be fixed first.

You know, another option would be to expose everconfirmed as a checkbox and have the default state be OPEN. Then any state could be confirmed or unconfirmed. I think that would be too complex in terms of workflow for most organizations, though. I suspect people would just too often forget to mark the checkbox.

Okay, here's all the changes that I think are necessary to support the new workflow. I've maintained backwards-compatibility for systems that don't want to change to the new workflow, in a few places. I've also included a script that will upgrade the old workflow to the new one (which also changes the bugs_activity table, to help out collectstats.pl). I suspect we may want to wait for the early 3.8 cycle to do this, instead of doing it right at the end of the 3.6 cycle, since it will be a somewhat controversial change.

Comment on attachment 418299 [details] [diff] [review] v1 >Index: template/en/default/pages/fields.html.tmpl >+The default status for queries is set to all open statuses except for >+[%+ display_value("bug_status", "INCONFIRMED") FILTER html %]. INCONFIRMED? :) I agree that we shouldn't take it so late in the game, especially because there can be many side-effects.

(In reply to comment #27) > I agree that we shouldn't take it so late in the game, especially because there > can be many side-effects. Yeah. I think we should review it soon so that it can go in right at the start of the 3.8 cycle, though.

FWIW, this is the #1 thing I want reviewed for 3.8, at the moment.

Comment on attachment 418299 [details] [diff] [review] v1 bitrotten

Fixed the bitrot.

Comment on attachment 428105 [details] [diff] [review] v2 >=== modified file 'Bugzilla/Bug.pm' >- if (ref $invocant && $new_status->name eq 'ASSIGNED' >+ if (ref $invocant && $new_status->name eq 'IN_PROGRESS' > && Bugzilla->params->{"usetargetmilestone"} > && Bugzilla->params->{"musthavemilestoneonaccept"} This change is problematic for all installations still using the old workflow. The musthavemilestoneonaccept parameter would suddenly stop working. Shouldn't we check for both ASSIGNED and IN_PROGRESS? >=== modified file 'Bugzilla/DB/Schema.pm' > allows_unconfirmed => {TYPE => 'BOOLEAN', NOTNULL => 1, >- DEFAULT => 'FALSE'}, >+ DEFAULT => 'TRUE'}, You must also fix the default value in Bugzilla/Install/DB.pm, isn't it? >=== modified file 'Bugzilla/Install.pm' >+sub init_workflow { >+ my $dbh = Bugzilla->dbh; >+ my $has_workflow = $dbh->selectrow_array('SELECT 1 FROM status_workflow'); >+ return if $has_workflow; As init_workflow() is called after update_table_definitions() in checksetup.pl, status_workflow will already be populated, and so you will always exit at this point. >+ foreach my $pair (STATUS_WORKFLOW) { >+ my $old_id = $pair->[0] ? $status_ids{$pair->[0]} : undef; >+ my $new_id = $status_ids{$pair->[1]}; >+ $dbh->do('INSERT INTO status_workflow (old_status, new_status) >+ VALUES (?,?)', undef, $old_id, $new_id); >+ } Unless you upgraded your workflow, most values will be undefined, introducing duplicated and invalid transitions. >=== modified file 'Bugzilla/Install/DB.pm' >-sub _initialize_workflow { >+sub _initialize_workflow_for_upgrade { >+ # We only populate the workflow here if we're upgrading from a version >+ # before 3.2. >+ return if $had_is_open; This doesn't make a lot of sense to me. Either we are upgrading from an installation older than 3.2 and we populate the status_workflow table, or we are upgrading from 3.2 - 3.6, and the status_workflow table is already populated, in which case we already skip the "populate the workflow table" code. In both cases is your $had_is_open check useless. >=== modified file 'checksetup.pl' > Bugzilla::Install::DB::update_table_definitions(\%old_params); >+Bugzilla::Install::init_workflow(); This call has no effect, per my previous comments. >=== added file 'contrib/convert-workflow.pl' >+Bugzilla::Status->check('VERIFIED'); Could you explain why you do this check here? Also, where do you make sure that the duplicate_or_move_bug_status parameter still points to a valid bug resolution (in case it was set to CLOSED)? >=== modified file 'contrib/jb2bz.py' >+ -s STATUS One of UNCONFIRMED, CONFIRMED, IN_PROGRESS, RESOLVED, VERIFIED, CLOSED s/CLOSED// ? >+ if a in ('UNCONFIRMED','CONFIRMED','IN PROGRESS','RESOLVED','VERIFIED','CLOSED'): Same question. >=== modified file 'template/en/default/admin/params/mta.html.tmpl' >- whinedays => "The number of days that we'll let a $terms.bug sit untouched in a NEW " _ >+ whinedays => "The number of days that we'll let a $terms.bug sit untouched in a CONFIRMED " _ Shouldn't you use display_value('bug_status', 'CONFIRMED') instead? >=== modified file 'template/en/default/email/whine.txt.tmpl' >+ [% urlbase %]buglist.cgi?bug_status=CONFIRMED&assigned_to=[% email %] This URL will only work for those using the new workflow.

(In reply to comment #32) > As init_workflow() is called after update_table_definitions() in checksetup.pl, > status_workflow will already be populated, and so you will always exit at this > point. Ah. The idea is that init_workflow, from now on, will always be the function that populates the workflow for brand-new installations. The code in Install::DB will only be used if you are upgrading from 3.2 or earlier. The problem is that we can't rely on the schema of bug_status and status_workflow to always be the same as it was during that point in Install::DB, but the *upgrade* always needs to happen right at that point. So the initial workflow population needs to happen after the database is fully upgraded, in Install::DB. This also allows us to have a very clean, simple piece of code in Bugzilla::Install to initialize the workflow for new installs. > >=== modified file 'template/en/default/admin/params/mta.html.tmpl' > > >- whinedays => "The number of days that we'll let a $terms.bug sit untouched in a NEW " _ > >+ whinedays => "The number of days that we'll let a $terms.bug sit untouched in a CONFIRMED " _ > > Shouldn't you use display_value('bug_status', 'CONFIRMED') instead? Well, I actually don't think that whineatnews is important enough to make it support both the old and new workflow. So I think that people should to customize, for the old workflow, or convert their workflow. I will definitely relnote it, though. > > > > >=== modified file 'template/en/default/email/whine.txt.tmpl' > > >+ [% urlbase %]buglist.cgi?bug_status=CONFIRMED&assigned_to=[% email %] > > This URL will only work for those using the new workflow.

Okay, this addresses the rest of your comments.

Comment on attachment 451717 [details] [diff] [review] v3 Okay, LpSolit said that he'd be more available now, so I'm hoping that this can be reviewed very soon? I really want to get this in before the hard freeze.

Fixed some bitrot.

Comment on attachment 453930 [details] [diff] [review] v4 >=== modified file 'Bugzilla/Install.pm' >+sub init_workflow { I see no reason to have a distinct init_workflow() subroutine from the one I wrote. Only STATUS_WORKFLOW differs, and so you should rather select the appropriate workflow based on whether we are upgrading or not. Moreover, your code makes no checks in case bug statuses differ from STATUS_WORKFLOW and for some reason checksetup.pl stopped before populating the status_workflow table. Also, RESOLVED/VERIFIED -> UNCONFIRMED should be explicitly allowed in the default workflow. I don't understand why we still need some magical hacks here. >=== modified file 'Bugzilla/Install/DB.pm' >+ # We only populate the workflow here if we're upgrading from a version >+ # before 3.2. >+ return if $had_is_open; If we are upgrading from 3.2 - 3.6 and something went wrong in the past for some reason, running checksetup.pl won't fix the DB as the code below this line will never be executed. I think this line should go away, and the correct default workflow be used based on whether we are upgrading or not, as mentioned above. >=== modified file 'Bugzilla/Search/Quicksearch.pm' > elsif ($words->[0] =~ /^[A-Z]+(,[A-Z]+)*$/) { >- # e.g. NEW,ASSI,REOP,FIX >+ # e.g. CON,IN_PR,FIX The regexp only accepts [A-Z]+, so IN_PR won't work due to the underscore, AFAIK. >=== modified file 'contrib/bugzilla-submit/bugzilla-submit' >- data['bug_status'] = 'NEW' >+ data['bug_status'] = 'CONFIRMED' You assume that we are using the new workflow, which probably won't be true for upgraded installations. >=== added file 'contrib/convert-workflow.pl' >+# We need the verified status to exist, in order to do the conversion. >+# This is the only status that we need that existed in both the old >+# workflow and the new one. >+Bugzilla::Status->check('VERIFIED'); Rather than throwing an error if VERIFIED doesn't exist, we should rather silently add it. >+ $dbh->do('UPDATE bugs SET bug_status = ? WHERE bug_status = ?', >+ undef, $to, $from); Nit: we used to add parens to make arguments more visible (undef is not used in placeholders here). >+ $dbh->do("UPDATE bugs_activity SET $what = ? >+ WHERE fieldid = ? AND $what = ?", >+ undef, $to, $status_field->id, $from); Nit: same here >+ else { >+ $dbh->do('UPDATE bug_status SET value = ? WHERE value = ?', >+ undef, $to, $from); Nit: and here. >=== modified file 'docs/en/images/bzLifecycle.xml' >=== modified file 'docs/en/xml/administration.xml' >=== modified file 'docs/en/xml/installation.xml' It's probably not clear that new installations have a different default workflow than older installations, and users may wonder why the documentation differs from what they really see. This is a problem, and the doc should be clearer. >=== modified file 'showdependencygraph.cgi' >- $stat ||= 'NEW'; >+ $stat ||= 'CONFIRMED'; Rather than assuming a bug status which probably doesn't exist, simply defaults to '' as this cannot happen anyway (bug_status is always defined for all bugs). I didn't test your patch yet, nor checked all places which could be affected by this change. So more comments may come later (once I'm happy with the patch :)).

(In reply to comment #37) > I see no reason to have a distinct init_workflow() subroutine from the one I > wrote. I explained the reasons that it is necessary in great detail, above. (This response also applies to your Install::DB comments.) One of the biggest reasons, also, is code simplicity. init_workflow is very simple, and is the only code we'll have to maintain from now on. The workflow upgrade code is very complex, and should no longer have to be updated (just like the rest of Install::DB does not have to be updated). > You assume that we are using the new workflow, which probably won't be true for > upgraded installations. True. I guess they won't be able to use bugzilla-submit, or they'd have to customize it. Remember that it wouldn't work anyway if somebody customized the workflow. It contained the default workflow before, so it should contain the default workflow now. > Rather than throwing an error if VERIFIED doesn't exist, we should rather > silently add it. Okay. There was some reason that I didn't do that, but I don't remember it now. If I remember, I'll comment. Otherwise, I'll just fix this. > Nit: we used to add parens to make arguments more visible (undef is not used in > placeholders here). Right, but I think the parens are unnecessary, and so should be left off, per perlstyle. > It's probably not clear that new installations have a different default > workflow than older installations, and users may wonder why the documentation > differs from what they really see. This is a problem, and the doc should be > clearer. Yeah, so should the rest of the almost-entirely-wrong Bugzilla Guide. Anyhow, it didn't account for custom workflows before, so I'm not too worried about that here.

Okay, I addressed all the issues that I didn't respond to in my comment above. For showdependencygraph, I just removed all three ||= statements--those fields are NOT NULL in the database--they will always have a value.

Comment on attachment 456105 [details] [diff] [review] v5 Review in progress, but so far, there are two places to fix: template/en/default/pages/quicksearch.html.tmpl, lines 100-101: 100: So, for example, searching on <kbd>stat:NEW</kbd> will find all 101: [%+ terms.bugs %] in the <kbd>NEW</kbd> status. Some fields have We should replace the example with NEW by an example with e.g. CONFIRMED. template/en/default/pages/fields.html.tmpl, line 91: 91: <b>[% display_value("bug_status", "NEW") FILTER html %]</b>, or s/NEW/CONFIRMED/.

comment 40 part 2: template/en/default/pages/fields.html.tmpl should get rid of ASSIGNED and REOPENED totally. The current field descriptions are confusing as they mix old and new bug statuses.

template/en/default/email/whine.txt.tmpl at line 46 should mention IN_PROGRESS instead of ASSIGNED, to match the new workflow.

Okay, this fixes all your comments. :-)

Comment on attachment 456124 [details] [diff] [review] v6 r=LpSolit

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/ modified checksetup.pl modified showdependencygraph.cgi modified whineatnews.pl modified Bugzilla/Bug.pm modified Bugzilla/Chart.pm modified Bugzilla/DB.pm modified Bugzilla/Install.pm modified Bugzilla/Migrate.pm modified Bugzilla/Status.pm modified Bugzilla/Config/Query.pm modified Bugzilla/DB/Schema.pm modified Bugzilla/Install/DB.pm modified Bugzilla/Migrate/Gnats.pm modified Bugzilla/Search/Quicksearch.pm added contrib/convert-workflow.pl modified contrib/jb2bz.py modified contrib/bugzilla-submit/bugzilla-submit modified docs/en/images/bzLifecycle.xml modified docs/en/xml/administration.xml modified docs/en/xml/installation.xml modified template/en/default/admin/params/bugchange.html.tmpl

Does this migrate old charts? I didn't find anything about this skimming the patch.

(In reply to comment #46) > Does this migrate old charts? I didn't find anything about this skimming the > patch. This is not done automatically by this script. But when the conversion is done, it displays the message: * You may want to run ./collectstats.pl --regenerate to regenerate data for the Old Charts system.

Hi, I come actually from bug 104049 to find out this is fixed by its blocker so late in the game. Anyway, can I still ask some questions? a) What happens when the user removes the "UNCONFIRMED" state from the default workflow? Does the CONFIRMED state remain? Maybe we should rename it to "CURRENT" in this case? Just wondering if I should open a new bug for that. b) If we introduced IN_PROGRESS, shouldn't we rename WORKSFORME to WORKS_FOR_ME?

(In reply to comment #47) All right, that does the trick. Thanks.

(In reply to comment #48) > a) What happens when the user removes the "UNCONFIRMED" state from the default > workflow? Does the CONFIRMED state remain? Maybe we should rename it to > "CURRENT" in this case? Just wondering if I should open a new bug for that. No, having CURRENT would be problematic as you can deactivate UNCONFIRMED on a per product basis, while bug statuses are used for all products at once. So this wouldn't be a good idea. If you don't use UNCONFIRMED at all and don't plan to, then you can rename CONFIRMED yourself. > b) If we introduced IN_PROGRESS, shouldn't we rename WORKSFORME to > WORKS_FOR_ME? I personally wouldn't want to do that without a very good reason. Renaming bug statuses and resolutions introduces many unexpected results, such as breaking series (new charts), old charts and saved searches. So this would generate more harm than a real benefit.

(In reply to comment #50) > (In reply to comment #48) > > a) What happens when the user removes the "UNCONFIRMED" state from the default > > workflow? Does the CONFIRMED state remain? Maybe we should rename it to > > "CURRENT" in this case? Just wondering if I should open a new bug for that. > > No, having CURRENT would be problematic as you can deactivate UNCONFIRMED on a > per product basis, while bug statuses are used for all products at once. So > this wouldn't be a good idea. If you don't use UNCONFIRMED at all and don't > plan to, then you can rename CONFIRMED yourself. Makes sense. > > > b) If we introduced IN_PROGRESS, shouldn't we rename WORKSFORME to > > WORKS_FOR_ME? > > I personally wouldn't want to do that without a very good reason. Renaming bug > statuses and resolutions introduces many unexpected results, such as breaking > series (new charts), old charts and saved searches. So this would generate more > harm than a real benefit. But we're talking about the default workflow here. Bugzillas that upgrade to a new version would be untouched.

What will be the impact of this change for languages other that English?

No impact. You just have to edit field-descs.none.tmpl, as you already do when you customize bug statuses since Bugzilla 3.2.

(In reply to comment #53) > No impact. You just have to edit field-descs.none.tmpl, as you already do when > you customize bug statuses since Bugzilla 3.2. However, if you're shipping a localization, you should ship with translations for both the old and the new statuses, by default.

Added to the release notes in bug 604256.

(In reply to comment #51) > > > b) If we introduced IN_PROGRESS, shouldn't we rename WORKSFORME to > > > WORKS_FOR_ME? > > > > I personally wouldn't want to do that without a very good reason. Renaming bug > > statuses and resolutions introduces many unexpected results, such as breaking > > series (new charts), old charts and saved searches. So this would generate more > > harm than a real benefit. > > But we're talking about the default workflow here. Bugzillas that upgrade to a > new version would be untouched. As I received no answer, I opened this issue as bug 630829.

For trunk, I've proposed a patch for UNCONFIRMED for bug 96616, since this patch doesn't solve the basic problem that UNCONFIRMED still cannot be removed for installs that don't use it. it is a simple fix, and hopefully it solves SOME of the unresolved comments here.