Closed
Bug 343338
Opened 19 years ago
Closed 18 years ago
Eliminate "my" variables from the root level of modules
Categories
(Bugzilla :: Bugzilla-General, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: mkanat, Assigned: mkanat)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
Certain modules have variables declared with "my" outside of any subroutine.
This is fine in CGI, but in mod_perl it could get weird.
Under mod_perl, "my" variables are only defined once, at the time Apache starts up. After that they're still defined, but they stick around. They live forever and never get reset.
So it's just safer to use constants, or something else, or just get rid of the variables if possible.
Assignee | ||
Comment 1•19 years ago
|
||
Just to make it clear, my intention with this bug is to make mod_cgi and mod_perl behave identically when it comes to these variables.
I did leave around the "my $columns" variables in certain bugs. Those will go away when we make those .pm files use Bugzilla::Object. In the mean time, it would have been a hassle to eliminate them, and it's not going to cause a big issue.
In fact, most of these would have been fine if left alone, but there's a chance that in the future somebody would come along and make some change, and then there would be a mysterious bug that only showed up on mod_perl, and not in mod_cgi. Or some customizer would have a problem that they couldn't figure out.
Attachment #227829 -
Flags: review?
Assignee | ||
Updated•19 years ago
|
Attachment #227829 -
Flags: review? → review?(justdave)
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•19 years ago
|
Attachment #227829 -
Flags: review?(justdave) → review?(LpSolit)
Assignee | ||
Comment 2•18 years ago
|
||
There was some bitrot; I fixed it.
Attachment #227829 -
Attachment is obsolete: true
Attachment #228884 -
Flags: review?(LpSolit)
Attachment #227829 -
Flags: review?(LpSolit)
Comment 3•18 years ago
|
||
Comment on attachment 228884 [details] [diff] [review]
v1.1
Looks good. Works correctly on a non-mod_perl installation. r=LpSolit
Attachment #228884 -
Flags: review?(LpSolit) → review+
Updated•18 years ago
|
Flags: approval?
Comment 4•18 years ago
|
||
Comment on attachment 228884 [details] [diff] [review]
v1.1
>Index: Bugzilla/Template.pm
>+sub _load_constants {
>+ use Bugzilla::Constants ();
Nit: just one thing: it doesn't make sense to use it in a routine AFAIK.
Updated•18 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 5•18 years ago
|
||
Great. This puts mod_perl into a testable condition.
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v <-- BugMail.pm
new revision: 1.84; previous revision: 1.83
done
Checking in Bugzilla/Config.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config.pm,v <-- Config.pm
new revision: 1.65; previous revision: 1.64
done
Checking in Bugzilla/FlagType.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v <-- FlagType.pm
new revision: 1.31; previous revision: 1.30
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v <-- Template.pm
new revision: 1.56; previous revision: 1.55
done
Checking in Bugzilla/Token.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Token.pm,v <-- Token.pm
new revision: 1.46; previous revision: 1.45
done
Checking in Bugzilla/Search/Quicksearch.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search/Quicksearch.pm,v <-- Quicksearch.pm
new revision: 1.11; previous revision: 1.10
done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•