Closed Bug 208699 Opened 21 years ago Closed 21 years ago

Move Throw{Code,Template}Error into Error.pm

Categories

(Bugzilla :: Bugzilla-General, enhancement, P1)

2.17.4
enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: bbaetz, Assigned: bbaetz)

References

Details

Attachments

(1 file, 2 obsolete files)

The remaining Throw functions need to get moved into a module. This requires me to do what I did to ThrowUserError, and have callers pass in params, rather then using the 'global' vars hash. Note that this removes the current case of dumping passed explicitly to the end of the message for a code error. All cases where this was done (except one, which was probably an oversight, and I made teh error message do it) had this in the main error text anyway. I left the use of the variables key to handle that, so it can still be used if someone wants to for whatever reason.
Attached patch patch (obsolete) — Splinter Review
OK, here we go. Its streightforward. It may clash slightly with gerv's filter patch and/or my dirname patch. Merging should be trivial, though.
Attachment #125206 - Flags: review?(gerv)
Blocks: bz-globals
+++ attachment.cgi 9 Jun 2003 03:10:24 -0000 > $vars->{'contenttypemethod'} = $::FORM{'contenttypemethod'}; > - ThrowCodeError("illegal_content_type_method"); > + ThrowCodeError("illegal_content_type_method", > + { contenttypemethod => $::FORM{'contenttypemethod'} }); Don't you want to remove the $vars line? I think this line is then no longer needed. Bugzilla/Error.pm > +manaully constructs urls without correct parameters. "manually" >+This function's behavious "behavior" ?
Yeah, I'll make those changes. Thanks.
Attached patch v2 (obsolete) — Splinter Review
Fixes bitrot.
Attachment #125206 - Attachment is obsolete: true
Attachment #125206 - Attachment is obsolete: false
Attachment #125206 - Flags: review?(gerv)
Attachment #130639 - Flags: review?(justdave)
Attached patch Patch v3Splinter Review
save as v2, but removes bitrot.
Attachment #125206 - Attachment is obsolete: true
Attachment #130639 - Attachment is obsolete: true
Severity: normal → enhancement
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → Bugzilla 2.18
Attachment #130639 - Flags: review?(justdave)
Comment on attachment 131378 [details] [diff] [review] Patch v3 Previous behavior of the ThrowCodeError routine was to take the extra variable hash passed on the end and make that be the 'variables' hash that is displayed under the error box. Your new version no longer does this, instead requiring a separate "variables" hash to be explicitly declared by the caller. However, nothing is done here to convert the calls that were already using this to comply with the new requirement. That's the only thing I can find wrong with this though. :) Otherwise it looks great.
Attachment #131378 - Flags: review-
Comment on attachment 131378 [details] [diff] [review] Patch v3 OK, based on discussion in IRC, it appears none of the places that were calling it this way previously really had a need to do so. Since that was my only objection, consider this reviewed. :)
Attachment #131378 - Flags: review- → review+
Flags: approval+
Checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Note: I think you missed adding a "use Bugzilla::Error" to Series.pm here (bug 221160). Never mind :-) Gerv
Actually, that's a bit of a slur - all the others had &:: in front of them except one, so it was my fault really. Gerv
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: