-
Notifications
You must be signed in to change notification settings - Fork 6
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
chore(lodash): [RO-26515] Removes unneeded lodash deps with native replacements #105
Conversation
module.exports = function (Promise) { | ||
class FixtureCache { | ||
|
||
constructor() { | ||
this.cache = {}; | ||
|
||
_bindAll(this, ['set', 'get', 'getOrPromise', 'setAsync']); |
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.
getOrPromise is not called in an async method in the class, so context is not needed
module.exports = function (Promise, fsPromise, JSON) { | ||
class Utils { | ||
|
||
constructor() { | ||
_bindAll(this, ['readFile', 'readJsonFile']); |
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.
Not called within an async method, so removing.
module.exports = function (path, Promise, fsPromise, FixtureCache, Logger) { | ||
class FixtureUtil { | ||
|
||
constructor(fixturesPath) { | ||
_bindAll(this, ['get', 'readAndProcessFile', 'startFileRead']); |
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.
Not called within an async method, so removing.
@@ -1,9 +1,7 @@ | |||
const _compact = require('lodash.compact'); | |||
const _map = require('lodash.map'); | |||
const _invokeMap = require('lodash.invokemap'); |
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 believe the _invokeMap
usage in this file could be replaced with
return self._pluginInstances.map(plugin => plugin.end());
Not sure if this is the only usage of _invokeMap
though
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 believe so but it's not low-hanging fruit. I will take a look at it in the next set of PRs. Coverage is not great to catch is so I don't want to commit to writing additional tests and delaying the main need of the project right now.
No description provided.