Last Comment Bug 486292 - Change the default workflow
: Change the default workflow
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: 3.5
: All All
: P1 enhancement (vote)
: Bugzilla 4.0
Assigned To: Max Kanat-Alexander
: default-qa
:
Mentors:
http://groups.google.com/group/mozill...
Depends on: 162060
Blocks: 104049 577956
  Show dependency treegraph
 
Reported: 2009-04-01 04:10 PDT by Max Kanat-Alexander
Modified: 2011-02-16 07:28 PST (History)
16 users (show)
mkanat: approval+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Work In Progress (5.90 KB, patch)
2009-09-01 23:27 PDT, Max Kanat-Alexander
no flags Details | Diff | Splinter Review
v1 (36.48 KB, patch)
2009-12-17 16:39 PST, Max Kanat-Alexander
LpSolit: review-
Details | Diff | Splinter Review
v2 (26.96 KB, patch)
2010-02-21 16:26 PST, Max Kanat-Alexander
LpSolit: review-
Details | Diff | Splinter Review
v3 (27.55 KB, patch)
2010-06-16 14:32 PDT, Max Kanat-Alexander
no flags Details | Diff | Splinter Review
v4 (27.44 KB, patch)
2010-06-24 17:31 PDT, Max Kanat-Alexander
LpSolit: review-
Details | Diff | Splinter Review
v5 (27.41 KB, patch)
2010-07-05 15:21 PDT, Max Kanat-Alexander
no flags Details | Diff | Splinter Review
v6 (31.28 KB, patch)
2010-07-05 17:37 PDT, Max Kanat-Alexander
LpSolit: review+
Details | Diff | Splinter Review

Description Max Kanat-Alexander 2009-04-01 04:10:04 PDT
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.
Comment 1 Marc Schumann [:Wurblzap] 2009-04-01 04:23:11 PDT
Seconded.
Comment 2 Byron Jones ‹:glob› [PTO until 2017-01-09] 2009-04-01 04:37:52 PDT
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.
Comment 3 Frédéric Buclin 2009-04-01 04:49:40 PDT
s/CONFIRMED/OPEN/ ?
Comment 4 Byron Jones ‹:glob› [PTO until 2017-01-09] 2009-04-01 06:04:10 PDT
yeah, i like OPEN in place of CONFIRMED
Comment 5 Gervase Markham [:gerv] 2009-04-01 06:38:48 PDT
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
Comment 6 David Lawrence [:dkl] 2009-04-01 11:45:18 PDT
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
Comment 7 Frédéric Buclin 2009-04-01 11:49:28 PDT
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.
Comment 8 David Lawrence [:dkl] 2009-04-01 12:00:15 PDT
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
Comment 9 Gordon P. Hemsley [:GPHemsley] 2009-04-01 13:11:22 PDT
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.
Comment 10 Byron Jones ‹:glob› [PTO until 2017-01-09] 2009-04-01 18:39:07 PDT
> 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.
Comment 11 Max Kanat-Alexander 2009-04-01 18:49:50 PDT
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.)
Comment 12 Gervase Markham [:gerv] 2009-04-06 03:25:26 PDT
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
Comment 13 Byron Jones ‹:glob› [PTO until 2017-01-09] 2009-04-06 04:29:03 PDT
> (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.
Comment 14 Gervase Markham [:gerv] 2009-04-06 04:34:04 PDT
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
Comment 15 Frédéric Buclin 2009-04-06 04:34:37 PDT
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.
Comment 16 Max Kanat-Alexander 2009-04-06 09:32:11 PDT
(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.
Comment 17 Max Kanat-Alexander 2009-09-01 22:24:21 PDT
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.
Comment 18 Max Kanat-Alexander 2009-09-01 23:27:42 PDT
Created attachment 398087 [details] [diff] [review]
Work In Progress

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 19 Max Kanat-Alexander 2009-09-01 23:37:30 PDT
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.
Comment 20 Gervase Markham [:gerv] 2009-09-02 02:56:22 PDT
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
Comment 21 Max Kanat-Alexander 2009-09-02 15:06:04 PDT
Hmm. Probably the solution there is to enable UNCONFIRMED by default.
Comment 22 Gervase Markham [:gerv] 2009-09-03 01:37:47 PDT
That would certainly alleviate the problem.

Have we yet disentangled UNCONFIRMED from the voting system?

Gerv
Comment 23 Max Kanat-Alexander 2009-09-03 13:45:39 PDT
(In reply to comment #22)
> Have we yet disentangled UNCONFIRMED from the voting system?

  It's being worked on: bug 162060
Comment 24 Frédéric Buclin 2009-09-27 05:01:47 PDT
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.
Comment 25 Max Kanat-Alexander 2009-11-08 16:45:05 PST
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.
Comment 26 Max Kanat-Alexander 2009-12-17 16:39:53 PST
Created attachment 418299 [details] [diff] [review]
v1

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 27 Frédéric Buclin 2009-12-17 16:46:00 PST
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.
Comment 28 Max Kanat-Alexander 2010-01-18 20:38:53 PST
(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.
Comment 29 Max Kanat-Alexander 2010-02-06 14:45:55 PST
FWIW, this is the #1 thing I want reviewed for 3.8, at the moment.
Comment 30 Frédéric Buclin 2010-02-21 14:36:22 PST
Comment on attachment 418299 [details] [diff] [review]
v1

bitrotten
Comment 31 Max Kanat-Alexander 2010-02-21 16:26:39 PST
Created attachment 428105 [details] [diff] [review]
v2

Fixed the bitrot.
Comment 32 Frédéric Buclin 2010-03-27 20:11:14 PDT
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.
Comment 33 Max Kanat-Alexander 2010-06-16 14:25:26 PDT
(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.
Comment 34 Max Kanat-Alexander 2010-06-16 14:32:52 PDT
Created attachment 451717 [details] [diff] [review]
v3

Okay, this addresses the rest of your comments.
Comment 35 Max Kanat-Alexander 2010-06-22 18:36:08 PDT
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.
Comment 36 Max Kanat-Alexander 2010-06-24 17:31:41 PDT
Created attachment 453930 [details] [diff] [review]
v4

Fixed some bitrot.
Comment 37 Frédéric Buclin 2010-06-28 17:20:49 PDT
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 :)).
Comment 38 Max Kanat-Alexander 2010-06-28 17:32:11 PDT
(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.
Comment 39 Max Kanat-Alexander 2010-07-05 15:21:43 PDT
Created attachment 456105 [details] [diff] [review]
v5

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 40 Frédéric Buclin 2010-07-05 16:54:40 PDT
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 41 Frédéric Buclin 2010-07-05 17:02:05 PDT
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.
Comment 42 Frédéric Buclin 2010-07-05 17:04:41 PDT
template/en/default/email/whine.txt.tmpl at line 46 should mention IN_PROGRESS instead of ASSIGNED, to match the new workflow.
Comment 43 Max Kanat-Alexander 2010-07-05 17:37:06 PDT
Created attachment 456124 [details] [diff] [review]
v6

Okay, this fixes all your comments. :-)
Comment 44 Frédéric Buclin 2010-07-05 17:41:52 PDT
Comment on attachment 456124 [details] [diff] [review]
v6

r=LpSolit
Comment 45 Max Kanat-Alexander 2010-07-05 17:44:03 PDT
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
Comment 46 Marc Schumann [:Wurblzap] 2010-07-06 00:21:40 PDT
Does this migrate old charts? I didn't find anything about this skimming the patch.
Comment 47 Frédéric Buclin 2010-07-06 04:44:11 PDT
(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.
Comment 48 Andrés G. Aragoneses 2010-07-06 05:11:00 PDT
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?
Comment 49 Marc Schumann [:Wurblzap] 2010-07-06 05:42:00 PDT
(In reply to comment #47)

All right, that does the trick. Thanks.
Comment 50 Frédéric Buclin 2010-07-06 05:45:45 PDT
(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.
Comment 51 Andrés G. Aragoneses 2010-07-06 07:32:56 PDT
(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.
Comment 52 mailtrader 2010-07-06 09:35:48 PDT
What will be the impact of this change for languages other that English?
Comment 53 Frédéric Buclin 2010-07-06 09:40:36 PDT
No impact. You just have to edit field-descs.none.tmpl, as you already do when you customize bug statuses since Bugzilla 3.2.
Comment 54 Max Kanat-Alexander 2010-07-06 09:47:10 PDT
(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.
Comment 55 Max Kanat-Alexander 2010-10-18 15:55:33 PDT
Added to the release notes in bug 604256.
Comment 56 Andrés G. Aragoneses 2011-02-02 03:29:35 PST
(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.
Comment 57 miketosh 2011-02-16 07:28:53 PST
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.

Note You need to log in before you can comment on or make changes to this bug.