Closed
Bug 191863
Opened 22 years ago
Closed 22 years ago
Clean up Bugzilla.pm
Categories
(Bugzilla :: Bugzilla-General, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: bbaetz, Assigned: bbaetz)
Details
Attachments
(1 file)
(deleted),
patch
|
justdave
:
review+
gerv
:
review+
|
Details | Diff | Splinter Review |
As discussed recently on developers and IRC, I've tidied up Bugzilla.pm a bit.
Nothing fancy. Note that the Bugzilla->* methods are still object-methods,
rather than subs. Theres no harm in this, and I have this vague idea to allow
'inheritance' of this at some point (via AUTOLOAD, similar to how File::Spec
works) for mod_perl-ish stuff. It also makes the plugin code a bit nicer, since
Template::Plugin::Procedural isn't in a stable TT release.
Assignee | ||
Comment 1•22 years ago
|
||
Assignee | ||
Updated•22 years ago
|
Attachment #113485 -
Flags: review?(justdave)
Comment 2•22 years ago
|
||
Comment on attachment 113485 [details] [diff] [review]
patch
OK, this looks good. I haven't tried it, but it looks pretty straightforward.
I'd feel a little better if someone could do a more thorough review (i.e. try
it) but I'm too wiped out right now to do that.
Attachment #113485 -
Flags: review?(justdave)
Attachment #113485 -
Flags: review?
Attachment #113485 -
Flags: review+
Assignee | ||
Comment 3•22 years ago
|
||
Comment on attachment 113485 [details] [diff] [review]
patch
Gerv?
Attachment #113485 -
Flags: review? → review?(gerv)
Assignee | ||
Comment 4•22 years ago
|
||
Comment on attachment 113485 [details] [diff] [review]
patch
Gerv?
Assignee | ||
Comment 5•22 years ago
|
||
Comment on attachment 113485 [details] [diff] [review]
patch
Gerv?
Comment 6•22 years ago
|
||
Comment on attachment 113485 [details] [diff] [review]
patch
>+Note that all C<Bugzilla> functionailty is method based; use C<Bugzilla->dbh>
>+rather than C<Bugzilla::DBH>. Nothing cares about this now, but don't rely on
>+that.
I don't quite understand this comment. Perhaps you could clarify it.
>Index: processmail
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/processmail,v
>retrieving revision 1.94
>diff -u -r1.94 processmail
>--- processmail 29 Dec 2002 07:06:41 -0000 1.94
>+++ processmail 4 Feb 2003 12:23:55 -0000
>@@ -28,6 +28,10 @@
> use strict;
> use lib ".";
>
>+use Bugzilla;
>+
>+Bugzilla->init({ TYPE => Bugzilla::TYPE_CMDLINE});
How does this work? I don't remember seeing this value looked at anywhere. Or
is it not in this patch (i.e. already present)?
Assuming you've caught all the relevant call sites, then r=gerv.
Gerv
Attachment #113485 -
Flags: review?(gerv)
Assignee | ||
Comment 7•22 years ago
|
||
I'm not sure how much clearer I can make it. (Apart from me having the case
wrong in the second example, that is - fixed locally)
What I wanted to say was that to get the current dbh, do:
my $dbh = Bugzilla->dbh();
not
my $dbh = Bugzilla::dbh();
nothing will notice if you use ::dbh ATM, but they may one day. They may not,
but its better to be consistant anyway. (They will notice when there are
arguments passed, obviously, but none of those subs take arguments) This isn't
likely to be a major issue, since people are likly to be copying and pasting the
code anyway.
except that that is what I said... Do you have a better way to say it? :)
Ignore the processmail hunk - that was left over from a previous version of the
patch. I did grep for Bugzilla, and "->instance", so I think I got everything.
Let me know what you want me to change, or I'll check this in when justdave
approves it.
Status: NEW → ASSIGNED
Flags: approval?
Target Milestone: --- → Bugzilla 2.18
Updated•22 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 8•22 years ago
|
||
Fixed
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•