-
Notifications
You must be signed in to change notification settings - Fork 42
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
added TransportStream option levelOnly as per request… #103
base: master
Are you sure you want to change the base?
added TransportStream option levelOnly as per request… #103
Conversation
Reduced some index.js calls and complexity for some of the eslint issues
* @param {mixed} info - TODO: add param description. | ||
* @returns {boolean} - TODO: add returns description. |
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.
PR leaves some TODOs open here.
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.
These TODOs are inherited from the original method that I extracted the code from. I just split the method out to fix the worst eslint issues that were breaking the npm run test
. I didn't want to assume what the descriptions are as I'm not super familiar with the code and project nomenclature.
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 had looked in the diff for that but didn't see a corresponding line being moved from elsewhere. Where was it extracted from?
In any case, those TODOs are added automatically by some JSDoc support tools in IDE packages. With the comment in the line above, these two lines can probably just be removed.
Thanks for starting in on this! |
@@ -20,6 +22,7 @@ const TransportStream = module.exports = function TransportStream(options = {}) | |||
|
|||
this.format = options.format; | |||
this.level = options.level; | |||
this.levelOnly = (options.levelOnly === true); // prevoius behaviour = not defined ~= false |
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.
this.levelOnly = (options.levelOnly === true); // prevoius behaviour = not defined ~= false | |
this.levelOnly = (options.levelOnly === true); // previous behaviour = not defined ~= false |
#40
Reduced some index.js calls and complexity for some of the eslint issues