Closed
Bug 328638
Opened 19 years ago
Closed 19 years ago
Remove @::legal_keywords and %::keywordsbyname
Categories
(Bugzilla :: Bugzilla-General, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: mkanat, Assigned: mkanat)
References
Details
Attachments
(1 file, 3 obsolete files)
20.83 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
The @::legal_keywords array is more-frequently-used and made differently than the rest of the @::legal arrays, so I'm going to remove it separately from the rest of them. I'm going to create a very simple Bugzilla::Keyword package, also.
Assignee | ||
Comment 1•19 years ago
|
||
And %::keywordsbyname isn't even used anywhere in Bugzilla.
Status: NEW → ASSIGNED
Summary: Remove @::legal_keywords → Remove @::legal_keywords and %::keywordsbyname
Assignee | ||
Comment 2•19 years ago
|
||
Okay, here's the patch to remove both of these vars. %::keywordsbyname was actually used, but only in GetKeywordIdFromName, which I replaced with various Bugzilla::Keyword object calls.
Attachment #213241 -
Flags: review?
Assignee | ||
Updated•19 years ago
|
Attachment #213241 -
Flags: review? → review?(wicked)
Comment 3•19 years ago
|
||
Comment on attachment 213241 [details] [diff] [review] v1 Sorry, this one is bitrotten too. :(
Attachment #213241 -
Flags: review?(wicked) → review-
Assignee | ||
Comment 4•19 years ago
|
||
Okay, fixed bitrot.
Attachment #213241 -
Attachment is obsolete: true
Attachment #213944 -
Flags: review?(wicked)
Comment 5•19 years ago
|
||
Comment on attachment 213944 [details] [diff] [review] v2 >Index: Bugzilla/Bug.pm >+use Bugzilla::Keyword; > sub use_keywords { >+ return Bugzilla::Keyword::keyword_count() > 0 ? 1 : 0; > } I think Bug::use_keywords() should be removed completely. It's only used once in ./template/en/default/bug/edit.html.tmpl and has nothing to do with bugs, btw. This is just a big hack. This would also avoid the Bug-Keyword dependency. >+++ Bugzilla/Keyword.pm 2006-03-03 15:32:57.000000000 -0800 >+sub get_all { It should be 'get_all_keywords', for consistency with other modules. >+ foreach my $id (@$ids) { >+ push @keywords, new Bugzilla::Keyword($id); >+ } The number of keywords can be large. You should implement Bugzilla::Keyword::new_from_list() to get them all at once.
Assignee | ||
Comment 6•19 years ago
|
||
Okay, good points for all of that. :-) I added new_from_list and eliminated use_keywords.
Attachment #213944 -
Attachment is obsolete: true
Attachment #214046 -
Flags: review?(LpSolit)
Attachment #213944 -
Flags: review?(wicked)
Comment 7•19 years ago
|
||
Comment on attachment 214046 [details] [diff] [review] v3 >Index: importxml.pl >+ if (!$keywordseen{$keyword_obj->id}) { >+ $key_sth->execute($id, $keyword_obj->id); >+ $keywordseen{$$keyword_obj->id} = 1; > } It should be $keywordseen{$keyword_obj->id} = 1;, (not $$). >Index: process_bug.cgi $vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count(); is missing. So when you change a bug, the keyword field is not displayed anymore because it now thinks we don't use keywords. >Index: Bugzilla/Bug.pm >+use Bugzilla::Keyword; You don't need it anymore. >+++ Bugzilla/Keyword.pm 2006-03-04 17:35:24.000000000 -0800 >+sub new_from_list { >+ @keywords = @{$dbh->selectall_arrayref( >+ "SELECT $columns FROM keyworddefs WHERE id IN (" >+ . join(',', @detainted_ids) . ")", {Slice=>{}}) || []}; Nit: I'm not sure you need to convert it to an array as you want to return a reference to it anyway.
Attachment #214046 -
Flags: review?(LpSolit) → review-
Assignee | ||
Comment 8•19 years ago
|
||
Okay, I made the changes you suggested.
Attachment #214046 -
Attachment is obsolete: true
Attachment #214226 -
Flags: review?(LpSolit)
Comment 9•19 years ago
|
||
Comment on attachment 214226 [details] [diff] [review] v4 r=LpSolit
Attachment #214226 -
Flags: review?(LpSolit) → review+
Updated•19 years ago
|
Flags: approval?
Comment 10•19 years ago
|
||
Comment on attachment 214226 [details] [diff] [review] v4 >+ $keywords = $dbh->selectall_arrayref( >+ "SELECT $columns FROM keyworddefs WHERE id IN (" >+ . join(',', @detainted_ids) . ")", {Slice=>{}}) || []; Nit: || [] is useless and should be removed.
Assignee | ||
Comment 12•19 years ago
|
||
Checking in buglist.cgi; /cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v <-- buglist.cgi new revision: 1.329; previous revision: 1.328 done Checking in colchange.cgi; /cvsroot/mozilla/webtools/bugzilla/colchange.cgi,v <-- colchange.cgi new revision: 1.52; previous revision: 1.51 done Checking in config.cgi; /cvsroot/mozilla/webtools/bugzilla/config.cgi,v <-- config.cgi new revision: 1.17; previous revision: 1.16 done Checking in enter_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v <-- enter_bug.cgi new revision: 1.130; previous revision: 1.129 done Checking in globals.pl; /cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl new revision: 1.358; previous revision: 1.357 done Checking in importxml.pl; /cvsroot/mozilla/webtools/bugzilla/importxml.pl,v <-- importxml.pl new revision: 1.50; previous revision: 1.49 done Checking in post_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v <-- post_bug.cgi new revision: 1.139; previous revision: 1.138 done Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.309; previous revision: 1.308 done Checking in query.cgi; /cvsroot/mozilla/webtools/bugzilla/query.cgi,v <-- query.cgi new revision: 1.157; previous revision: 1.156 done Checking in show_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/show_bug.cgi,v <-- show_bug.cgi new revision: 1.40; previous revision: 1.39 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.113; previous revision: 1.112 done RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Keyword.pm,v done Checking in Bugzilla/Keyword.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Keyword.pm,v <-- Keyword.pm initial revision: 1.1 done Checking in Bugzilla/Search.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v <-- Search.pm new revision: 1.124; previous revision: 1.123 done Checking in Bugzilla/Search/Quicksearch.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search/Quicksearch.pm,v <-- Quicksearch.pm new revision: 1.4; previous revision: 1.3 done Checking in template/en/default/bug/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.73; previous revision: 1.72 done I fixed the checkin nit only after I checked in the original: Checking in Bugzilla/Keyword.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Keyword.pm,v <-- Keyword.pm new revision: 1.2; previous revision: 1.1 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
•