Closed
Bug 1007605
Opened 10 years ago
Closed 9 years ago
Make "FIXED" non-fixed, by changing noresolveonopenblockers to define what the fixed resolution is
Categories
(Bugzilla :: Administration, task)
Bugzilla
Administration
Tracking
()
RESOLVED
FIXED
Bugzilla 6.0
People
(Reporter: gerv, Assigned: gerv)
References
Details
(Keywords: relnote)
Attachments
(2 files, 1 obsolete file)
13.33 KB,
patch
|
glob
:
review+
|
Details | Diff | Splinter Review |
777 bytes,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
At the moment, it's not permitted to delete or alter the name of the FIXED resolution (ChoiceInterface.pm around line 88): sub is_static { ... if ($self->field->name eq 'resolution') { return grep($_ eq $self->name, ('', 'FIXED', 'DUPLICATE')) ? 1 : 0; } ... } Grepping the code, the only reason for this I can find is that the FIXED resolution is hard-coded in the implementation of the "noresolveonopenblockers" param. We should change "noresolveonopenblockers" so that it's no longer a boolean, but instead a choice of resolutions, such that if you set it, it's not possible to set a bug to that chosen resolution without resolving all the open blockers. That way, we can un-hard-code "FIXED", and allow implementations to delete it or change its name. Gerv
Updated•10 years ago
|
Severity: normal → enhancement
OS: Linux → All
Hardware: x86 → All
Assignee | ||
Comment 1•10 years ago
|
||
There are a few knock-on effects, but it's all a fairly straightforward param upgrade. I think the added flexibility of being able to rename or change all resolutions apart from DUPLICATE is very helpful. Gerv
Comment on attachment 8531211 [details] [diff] [review] Patch v.1 Review of attachment 8531211 [details] [diff] [review]: ----------------------------------------------------------------- this generally looks good, just a few minor things to address. ::: Bugzilla/Config.pm @@ +213,5 @@ > } > > + if (exists $param->{'noresolveonopenblockers'}) { > + $new_params{'resolution_forbidden_with_open_blockers'} = > + $param->{'noresolveonopenblockers'} ? 'FIXED' : ""; nit: no need to wrap this line ::: Bugzilla/Config/BugChange.pm @@ +30,5 @@ > # If no closed state was found, use the default list above. > @closed_bug_statuses = @current_closed_states if scalar(@current_closed_states); > }; > > + my $resolution_field = new Bugzilla::Field({ name => 'resolution' }); please don't use old style perl constructors, and always use the cache where possible: Bugzilla::Field->new({ name => 'resolution, cache => 1 }) @@ +83,2 @@ > } ); > + remove trailing whitespace ::: Bugzilla/Config/Common.pm @@ +178,5 @@ > } > > +sub check_resolution { > + my $resolution = shift; > + my $resolution_field = new Bugzilla::Field({ name => 'resolution' }); same point here regarding ->new and caching @@ +181,5 @@ > + my $resolution = shift; > + my $resolution_field = new Bugzilla::Field({ name => 'resolution' }); > + # The empty resolution is included - it represents "no value" > + my @resolutions = map {$_->name} @{ $resolution_field->legal_values }; > + remove trailing whitespace ::: Bugzilla/Field/ChoiceInterface.pm @@ +92,1 @@ > ? 1 : 0; this would be better as return $self->name eq '' || $self->name eq 'DUPLICATE' ::: docs/en/rst/administration.rst @@ -210,5 @@ > database users than having a developer mark a bug "fixed" without > any comment as to what the fix was (or even that it was truly > fixed!) > > -noresolveonopenblockers you've removed documentation for this param, but didn't add docs for the param while replaces it. ::: docs/en/rst/api/core/v1/bugzilla.rst @@ -202,5 @@ > "maintainer" : "admin@example.com", > "maxattachmentsize" : "1000", > "maxlocalattachment" : "0", > "musthavemilestoneonaccept" : "0", > - "noresolveonopenblockers" : "0", the new parameter is returned, so it should be added to the example response @@ -240,5 @@ > * maintainer > * maxattachmentsize > * maxlocalattachment > * musthavemilestoneonaccept > -* noresolveonopenblockers similarly resolution_forbidden_with_open_blockers should be added here ::: template/en/default/admin/params/bugchange.html.tmpl @@ +36,5 @@ > > commentonduplicate => "If this option is on, the user needs to enter a short comment " _ > "if the $terms.bug is marked as duplicate.", > > + resolution_forbidden_with_open_blockers => "Don\'t allow $terms.bugs to be resolved with " _ nit: no need to escape the ' in Don't
Attachment #8531211 -
Flags: review?(glob) → review-
Assignee | ||
Comment 3•9 years ago
|
||
Review comments addressed. Gerv
Attachment #8531211 -
Attachment is obsolete: true
Attachment #8580120 -
Flags: review?(glob)
Comment on attachment 8580120 [details] [diff] [review] Patch v.2 Review of attachment 8580120 [details] [diff] [review]: ----------------------------------------------------------------- r=glob
Attachment #8580120 -
Flags: review?(glob) → review+
Assignee | ||
Comment 5•9 years ago
|
||
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla 2c82105..79ec299 master -> master Gerv
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 6•9 years ago
|
||
Comment on attachment 8580120 [details] [diff] [review] Patch v.2 >+++ b/Bugzilla/Config/BugChange.pm >+ my $resolution_field = Bugzilla::Field->new({ name => 'resolution', cache => 1 }); >+ # The empty resolution is included - it represents "no value" >+ my @resolutions = map {$_->name} @{ $resolution_field->legal_values }; checksetup.pl now crashes when installing Bugzilla for the first time, because the fielddefs table hasn't been populated yet, and so you get: Can't call method "legal_values" on an undefined value at Bugzilla/Config/BugChange.pm line 37.
Comment 7•9 years ago
|
||
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git a29f798..52ff6aa master -> master Unrelated to the crash, but related to the original patch: the description of the new parameter could be clearer and should specify that leaving this parameter empty disables the feature.
Attachment #8587966 -
Flags: review+
Assignee | ||
Comment 8•9 years ago
|
||
Sorry :-( Filed bug 1151396 for improvements to parameter text. Gerv
Comment 9•8 years ago
|
||
I updated the documentation now that FIXED is no longer a hardcoded resolution. (I also fixed a link pointing to landfill, so that the user can choose which Bugzilla version he wants to play with. Unrelated to this bug, I know. ;)) To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git f798c04..c688265 master -> master
You need to log in
before you can comment on or make changes to this bug.
Description
•