Closed
Bug 328438
Opened 19 years ago
Closed 19 years ago
Eliminate @::log_columns
Categories
(Bugzilla :: Bugzilla-General, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: mkanat, Assigned: mkanat)
References
Details
Attachments
(1 file, 2 obsolete files)
7.09 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
Right now the @::log_columns global variable is used for a lot of purposes, in a few files. Basically, it lists what the "important" fields of the bugs table are. Basically, as far as I can tell, it's called log_columns because it lists which columns should be "logged" into the activity table, or which ones should be mentioned in email. It really seems like we should just have some boolean field in the fielddefs table to accomplish this, but I'm not sure what it would be called. Perhaps for now we'll just make a subroutine in Bugzilla::Bug called editable_bug_fields().
Assignee | ||
Comment 1•19 years ago
|
||
This patch is correct in every way except that Bugzilla::BugMail is actually unable to use Bugzilla::Bug, because of a dependency loop.
Assignee | ||
Comment 2•19 years ago
|
||
Comment on attachment 213032 [details] [diff] [review] v1 Okay, we've fixed the dep loop, so this now works fine. Just make sure that I didn't miss adding any table locks anywhere.
Attachment #213032 -
Attachment description: Non-Working Patch → v1
Attachment #213032 -
Flags: review?(wicked)
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Comment 3•19 years ago
|
||
Comment on attachment 213032 [details] [diff] [review] v1 Sorry, this one is bitrotten too. :(
Attachment #213032 -
Flags: review?(wicked) → review-
Assignee | ||
Comment 4•19 years ago
|
||
Okay, I fixed the bitrot.
Attachment #213032 -
Attachment is obsolete: true
Attachment #213937 -
Flags: review?(wicked)
Comment 5•19 years ago
|
||
Comment on attachment 213937 [details] [diff] [review] v2 When changing a bug, it crashes with: undef error - Undefined subroutine &Bugzilla::BugMail::editable_bug_fields called at Bugzilla/BugMail.pm line 141.
Attachment #213937 -
Flags: review?(wicked) → review-
Comment 6•19 years ago
|
||
Now that we have a separate Bugzilla::Mailer module, I think this patch would now work correctly. You probably just have to unbitrot it. About @::log_columns in process_bug.cgi: as we use it several times in this script, probably should you call it *before* locking tables and store it in a variable, so that you can freely use it without having to lock bz_schema.
Assignee | ||
Comment 7•19 years ago
|
||
Okay, I fixed the bitrot, and I implemented LpSolit's suggestion. I also tested changing a bug, and it works fine.
Attachment #213937 -
Attachment is obsolete: true
Attachment #225799 -
Flags: review?(LpSolit)
Comment 8•19 years ago
|
||
Comment on attachment 225799 [details] [diff] [review] v3 r=LpSolit
Attachment #225799 -
Flags: review?(LpSolit) → review+
Updated•19 years ago
|
Flags: approval? → approval+
Comment 9•19 years ago
|
||
Max, I did the checkin myself as I need it to avoid conflicts with my patch about @::legal_*. Checking in globals.pl; /cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl new revision: 1.369; previous revision: 1.368 done Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.323; previous revision: 1.322 done Checking in query.cgi; /cvsroot/mozilla/webtools/bugzilla/query.cgi,v <-- query.cgi new revision: 1.162; previous revision: 1.161 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.120; previous revision: 1.119 done Checking in Bugzilla/BugMail.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v <-- BugMail.pm new revision: 1.77; previous revision: 1.76 done Checking in docs/xml/customization.xml; /cvsroot/mozilla/webtools/bugzilla/docs/xml/customization.xml,v <-- customization.xml new revision: 1.33; previous revision: 1.32 done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•