Closed
Bug 173622
Opened 22 years ago
Closed 22 years ago
Move template handling code into a module
Categories
(Bugzilla :: Bugzilla-General, defect)
Bugzilla
Bugzilla-General
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: bbaetz, Assigned: bbaetz)
References
Details
Attachments
(1 file, 4 obsolete files)
31.56 KB,
patch
|
justdave
:
review+
bugreport
:
review+
|
Details | Diff | Splinter Review |
We should move the template/vars stuff into a module. This is because:
- we shouldn't be running the code from a require'd file
- global variables suck
- its cleaner/nicer/more efficient
I have bits of this done, but its not completed.
Assignee | ||
Updated•22 years ago
|
Blocks: bz-globals
Assignee | ||
Comment 1•22 years ago
|
||
I've been running with this for about a month now.
The only substantial change in the patch is to use cgi.user_agent instead of
pushing a user_agent var all the time. This is a CGI instance, not a
Bugzilla::CGI instance. Not that it matters in this case; if anyone ever does
care we'd have to write a wrpaper plugin. Its possible I may do this as part of
the relogin + confirm stuff, whic currently uses FORM/MFORM, but thats not this
bug.
The rest is just code movement. Note that the 'default' vars stuff is now a
VARAIBLES option to the template, rather than in the vars hash, which makes it
easier to remove the global-ness of that later.
Assignee | ||
Comment 2•22 years ago
|
||
Comment on attachment 106788 [details] [diff] [review]
v1
myk - you originally did the template stuff.
There will be a minor change or two to the actual template defn to deal with
other patches, but thats jsut going to be applying to Template.pm what is
applied to globals.pl
The top hunk was needed, but please don't ask me to remember why. Its 'correct'
in any event, though
Attachment #106788 -
Flags: review?(myk)
Assignee | ||
Comment 3•22 years ago
|
||
OK, this uses a different scheme, as I discussed on developers@b.o
Theres now a semi-persistable Bugzilla object. Bugzilla.pm has more POD than
code, so it should be easy to comment on the design for that.
Additional changes from the previous patch are:
- to use this new object in the templates via a plugin (which will mean
creating a new directory - any objections?).
- collectstats is changed to call duplicates.cgi with GATEWAY_INTERFACE set, so
that it has a CGI object
- Bugzilla::Template no longer caches the template, since Bugzilla.pm is doing
that for us. That makes the code a bit nicer too, btw
This patch is larger than it looks - a lot of stuff is just movement of code
Attachment #106788 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #106788 -
Flags: review?(myk)
Assignee | ||
Updated•22 years ago
|
Attachment #106982 -
Flags: review?(myk)
Assignee | ||
Updated•22 years ago
|
Attachment #106982 -
Flags: review?(justdave)
Assignee | ||
Comment 4•22 years ago
|
||
Fixes bitrot from both the help checkin, and the time filter changes.
Attachment #106982 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #106982 -
Attachment is obsolete: false
Attachment #106982 -
Flags: review?(myk)
Attachment #106982 -
Flags: review?(justdave)
Assignee | ||
Updated•22 years ago
|
Attachment #107668 -
Flags: review?(myk)
Assignee | ||
Updated•22 years ago
|
Attachment #107668 -
Flags: review?(justdave)
Assignee | ||
Comment 5•22 years ago
|
||
Weekly update to fix more cvs conflicts.
Attachment #106982 -
Attachment is obsolete: true
Attachment #107668 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #107668 -
Attachment is obsolete: false
Attachment #107668 -
Flags: review?(myk)
Attachment #107668 -
Flags: review?(justdave)
Assignee | ||
Updated•22 years ago
|
Attachment #108017 -
Flags: review?(myk)
Assignee | ||
Updated•22 years ago
|
Attachment #108017 -
Flags: review?(justdave)
Updated•22 years ago
|
Attachment #108017 -
Flags: review?(bugreport)
Assignee | ||
Comment 6•22 years ago
|
||
Fix cvs conflicts
Attachment #107668 -
Attachment is obsolete: true
Attachment #108017 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #109337 -
Flags: review?(justdave)
Assignee | ||
Updated•22 years ago
|
Attachment #108017 -
Flags: review?(myk)
Attachment #108017 -
Flags: review?(justdave)
Attachment #108017 -
Flags: review?(bugreport)
Assignee | ||
Updated•22 years ago
|
Attachment #109337 -
Flags: review?(bugreport)
Comment 7•22 years ago
|
||
Comment on attachment 109337 [details] [diff] [review]
v5
my head hurts trying to look at the code in this patch, but bbaetz tells me
it's mostly existing code just moving around. I don't see anything in it
that's jumping out and screaming at me. I installed this patch on Syndicomm 2
days ago, and it's actually been getting heavy use because we're in the middle
of a cross-continent server move and we're using it for a task list. Haven't
noticed any problems.
Attachment #109337 -
Flags: review?(justdave) → review+
Comment 8•22 years ago
|
||
Oh, and I forgot to mention, we had a checkin on the 16th that broke
show_bug.cgi on Perl 5.6.0 because of weird taint problems... this patch fixes
that, so it would be really really nice to have this in before we roll 2.17.2
tomorrow morning :-)
Comment 10•22 years ago
|
||
Comment on attachment 109337 [details] [diff] [review]
v5
r=joel
I've diverged enough with my production site that I can only try this on my
little one, but it seems pretty innocuous.
Attachment #109337 -
Flags: review?(bugreport)
Updated•22 years ago
|
Flags: approval+
Assignee | ||
Comment 11•22 years ago
|
||
Checked in!
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•22 years ago
|
||
Somehow the checkin message ended up as '/tmp/cvshseAPa', so I cvs admin -m'd
the files to fix that. Bonsai won't pick that up, though.
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
•