Closed
Bug 143743
Opened 22 years ago
Closed 22 years ago
Warning generated in dependency tree
Categories
(Bugzilla :: Bugzilla-General, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.16
People
(Reporter: jouni, Assigned: jouni)
Details
Attachments
(1 file, 1 obsolete file)
570 bytes,
patch
|
bbaetz
:
review+
justdave
:
review+
|
Details | Diff | Splinter Review |
The dependency tree template generates a warning because not all dependencies have dependencies themselves, and there is one condition in display_tree block which doesn't check for this. The warning message: Argument "" isn't numeric in numeric gt (>) at data/template/en/default/bug/dependency-tree.html.tmpl line 284 (#1) (W numeric) The indicated string was fed as an argument to an operator that expected a numeric value instead. If you're fortunate the message will identify which operator was so unfortunate. [Sat May 11 19:00:28 2002] Patch follows. Ccing Burnus and Myk (template authors) for comments/review.
Assignee | ||
Comment 1•22 years ago
|
||
Adds one condition, killing the warning.
Assignee | ||
Comment 2•22 years ago
|
||
The fix should be trivial enough, so proposing milestone 2.16 to get this into the branch as well.
Comment 3•22 years ago
|
||
I wonder if it would be better for every bug record to have a dependencies field.
Comment 4•22 years ago
|
||
uh, is that anything like the "keyword cache"? I'm not particularly fond of that idea...
Comment 5•22 years ago
|
||
No, I mean that when generating the Perl data structure representing the dependency tree, we should initialize each Perl bug record with an empty list of dependencies. Then we wouldn't have to check whether or not the list of dependencies exists, we could just check its size (like we currently do).
Comment 6•22 years ago
|
||
Oh, for the values passed to the template you mean, not the DB schema. Yeah, that makes perfect sense. Bug.pm would fit well for feeding bugs to templates ya know... are we using it? (just an off-topic aside :)
Assignee | ||
Comment 7•22 years ago
|
||
Before implementing any solution, the true meaning of these undef'd fields in template data structures should be considered. This is not the first bug (I think it's the fourth for 2.16) I've filed with exactly the same characteristics. We should consider whether "nulls" should be passed as empty lists or undef'd variables and then make it a standard coding practice. The check "foo && foo.size > 0" is actually redundant in most of the cases, since either one of the conditions would suffice if the code was designed uniformly. So, there should be a design document either stating that lists with empty meaning should be passed as undef'd or a document stating that there should always be a list, empty if appropriate. This way all template authors could trust on certain convention being used. I still think this bug should be fixed using my patch, since it's quick and easy. If something is decided on the more generic issue presented above, a new bug should be filed to convert all the existing empty list code to use whichever method is chosen.
Comment 8•22 years ago
|
||
Comment on attachment 83210 [details] [diff] [review] Patch As it turns out, the code in showdependencytree.cgi already initializes the Perl bug records with empty arrays (line 170), but Perl returns the undefined value when evaluating an empty array in a scalar context. I think the Template Toolkit should be handling this situation by returning numeric zero instead of the undefined value in this situation. I'll report this bug to the Template Toolkit folks. In the meantime, Jouni's patch is the right workaround, so 2xr=myk.
Attachment #83210 -
Flags: review+
Comment 9•22 years ago
|
||
Ok, the problem turns out to be that I'm initializing the array reference with an empty list instead of an array reference. This patch fixes the problem.
Attachment #83210 -
Attachment is obsolete: true
Comment 10•22 years ago
|
||
Comment on attachment 83603 [details] [diff] [review] patch v2: fixes problem in showdependencytree.cgi r=bbaetz, per discussion on TT list.
Attachment #83603 -
Flags: review+
Comment 11•22 years ago
|
||
Comment on attachment 83603 [details] [diff] [review] patch v2: fixes problem in showdependencytree.cgi r= justdave
Attachment #83603 -
Flags: review+
Comment 12•22 years ago
|
||
Checking in bztip/showdependencytree.cgi; /cvsroot/mozilla/webtools/bugzilla/showdependencytree.cgi,v <-- showdependencytree.cgi new revision: 1.18; previous revision: 1.17 done Checking in bz216/showdependencytree.cgi; /cvsroot/mozilla/webtools/bugzilla/showdependencytree.cgi,v <-- showdependencytree.cgi new revision: 1.17.2.1; previous revision: 1.17 done
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
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
•