Opened 9 years ago
Closed 9 years ago
#14955 closed task (fixed)
dojox/analytics/consoleMessages plugin missing "var"
Reported by: | mm | Owned by: | dylan |
---|---|---|---|
Priority: | high | Milestone: | 1.8 |
Component: | Dojox | Version: | 1.7.2 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description (last modified by )
define(["dojo/_base/lang","../_base", "dojo/_base/config", "dojo/aspect" ], function(lang, dxa, config, aspect){ /*===== dxa = dojox.analytics; aspect = dojo.aspect; =====*/ consoleMessages = lang.getObject("dojox.analytics.plugins.consoleMessages", true); // summary: // plugin to have analyitcs return the base info dojo collects this.addData = lang.hitch(dxa, "addData", "consoleMessages"); var lvls = config["consoleLogFuncs"] || ["error", "warn", "info", "rlog"]; if(!console){ console = {}; } for(var i = 0; i < lvls.length; i++){ if(console[lvls[i]]){ aspect.after(console, lvls[i], lang.hitch(this, "addData", lvls[i]),true); }else{ console[lvls[i]] = lang.hitch(this, "addData", lvls[i]); } } return consoleMessages; });
Several questions for this source code:
Missing var in front of consoleMessages (global variable ? Purpose ? I hope not)
Using lang.getObject(, true) instead of declare() (purpose ? Avoiding dependency on base/_declare and preferring lax style instead ?)
Repeated lvls[i] lookup and lengthy code ?
Change History (13)
comment:2 Changed 9 years ago by
Component: | General → Dojox |
---|---|
Description: | modified (diff) |
Owner: | set to Adam Peller |
comment:3 Changed 9 years ago by
Owner: | changed from Adam Peller to Dustin Machi |
---|---|
Status: | new → assigned |
comment:4 Changed 9 years ago by
mm, I'm not sure who you are? We can't take code unless you've filed a CLA, although we can add the missing "var" that you mentioned.
As for lang.getObject("dojox.analytics.plugins.consoleMessages", true)
, that's a typical way to create a global variable, but will disappear in 2.0 when we get rid of global variables, and will be replaced by something like
var consoleMessages = {};
comment:5 Changed 9 years ago by
Summary: | dojox/analytics/consoleMessages plugin → dojox/analytics/consoleMessages plugin missing "var" |
---|
comment:7 Changed 9 years ago by
I checked the lists and there is no one named "mm". What name are you listed under?
comment:11 Changed 9 years ago by
Milestone: | tbd → 1.8 |
---|---|
Priority: | undecided → high |
Resolution: | fixed |
Status: | closed → reopened |
I've accepted most of this patch, but not the change for
(console={}); |
Mostly because our post-commit hook doesn't like it, and I didn't want to override it for a change that is minor.
comment:12 Changed 9 years ago by
Owner: | changed from Dustin Machi to dylan |
---|---|
Status: | reopened → assigned |
comment:13 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
proposed rewrite (comments welcomed)