-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
prune global-level trace attributes that are already defined in a trace #3158
Changes from 1 commit
fbfe638
f168ddc
7999517
7f60f6e
930e4c8
b7679a6
d501bb0
cd0230e
da03ceb
397870b
55881c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -460,11 +460,22 @@ function getTraceAttributes(type) { | |
// make 'type' the first attribute in the object | ||
attributes.type = null; | ||
|
||
|
||
var copyBaseAttributes = extendDeepAll({}, baseAttributes), | ||
copyModuleAttributes = extendDeepAll({}, _module.attributes); | ||
|
||
// prune global-level trace attributes that are already defined in a trace | ||
exports.crawl(copyModuleAttributes, function(attr, attrName, attrs, level, fullAttrString) { | ||
delete copyBaseAttributes[fullAttrString]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
// Prune undefined attributes | ||
if(attr === undefined) delete copyModuleAttributes[fullAttrString]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice touch here, that way we won't have to rely on |
||
}); | ||
|
||
// base attributes (same for all trace types) | ||
extendDeepAll(attributes, baseAttributes); | ||
extendDeepAll(attributes, copyBaseAttributes); | ||
|
||
// module attributes | ||
extendDeepAll(attributes, _module.attributes); | ||
extendDeepAll(attributes, copyModuleAttributes); | ||
|
||
// subplot attributes | ||
if(basePlotModule.attributes) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have gone out of style in our codebase. It might be worth adding an
eslint
rule for this. Fromhttps://eslint.org/docs/rules/one-var#initialized-and-uninitialized
looks like
"one-var": { "initialized": "never", "uninitialized": "consecutive" }
is exactly what most of the codebase does already.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed this in f168ddc and opened an issue (#3162) so I don't forget about adding an
eslint
rule for this.