Closed Bug 173622 Opened 19 years ago Closed 18 years ago

Move template handling code into a module

Categories

(Bugzilla :: Bugzilla-General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: bbaetz, Assigned: bbaetz)

References

Details

Attachments

(1 file, 4 obsolete files)

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.
Depends on: 171493
Target Milestone: --- → Bugzilla 2.18
Blocks: bz-globals
Attached patch v1 (obsolete) — Splinter Review
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.
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)
Attached patch take 2 (obsolete) — Splinter Review
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
Attachment #106788 - Flags: review?(myk)
Attachment #106982 - Flags: review?(myk)
Attachment #106982 - Flags: review?(justdave)
Attached patch v3 (obsolete) — Splinter Review
Fixes bitrot from both the help checkin, and the time filter changes.
Attachment #106982 - Attachment is obsolete: true
Attachment #106982 - Attachment is obsolete: false
Attachment #106982 - Flags: review?(myk)
Attachment #106982 - Flags: review?(justdave)
Attachment #107668 - Flags: review?(myk)
Attachment #107668 - Flags: review?(justdave)
Attached patch v4 (obsolete) — Splinter Review
Weekly update to fix more cvs conflicts.
Attachment #106982 - Attachment is obsolete: true
Attachment #107668 - Attachment is obsolete: true
Attachment #107668 - Attachment is obsolete: false
Attachment #107668 - Flags: review?(myk)
Attachment #107668 - Flags: review?(justdave)
Attachment #108017 - Flags: review?(myk)
Attachment #108017 - Flags: review?(justdave)
Attached patch v5Splinter Review
Fix cvs conflicts
Attachment #107668 - Attachment is obsolete: true
Attachment #108017 - Attachment is obsolete: true
Attachment #109337 - Flags: review?(justdave)
Attachment #108017 - Flags: review?(myk)
Attachment #108017 - Flags: review?(justdave)
Attachment #108017 - Flags: review?(bugreport)
Attachment #109337 - Flags: review?(bugreport)
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+
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 :-)
Joel: see comment 8
OS: Linux → All
Hardware: PC → All
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)
Checked in!
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
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.
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.