Warning generated in dependency tree

RESOLVED FIXED in Bugzilla 2.16

Status

()

Bugzilla
Bugzilla-General
RESOLVED FIXED
15 years ago
4 years ago

People

(Reporter: Jouni Heikniemi, Assigned: Jouni Heikniemi)

Tracking

2.15
Bugzilla 2.16

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

15 years ago
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

15 years ago
Created attachment 83210 [details] [diff] [review]
Patch

Adds one condition, killing the warning.
(Assignee)

Comment 2

15 years ago
The fix should be trivial enough, so proposing milestone 2.16 to get this into
the branch as well.
Status: NEW → ASSIGNED
Keywords: patch, review
Target Milestone: --- → Bugzilla 2.16
I wonder if it would be better for every bug record to have a dependencies field.
uh, is that anything like the "keyword cache"?  I'm not particularly fond of
that idea...
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).
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

15 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 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+
Created attachment 83603 [details] [diff] [review]
patch v2: fixes problem in showdependencytree.cgi

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 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 on attachment 83603 [details] [diff] [review]
patch v2: fixes problem in showdependencytree.cgi

r= justdave
Attachment #83603 - Flags: review+
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
Last Resolved: 15 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.