Replaced lodash pullAllWith with Array.filter() #7861
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7861 +/- ##
==========================================
- Coverage 88.41% 88.41% -0.01%
==========================================
Files 244 244
Lines 9047 9046 -1
==========================================
- Hits 7999 7998 -1
Misses 1048 1048
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
@calganaygun great thanks for giving that a spin. Still please see my minor suggestion
| @@ -25,7 +25,7 @@ module.exports = { | |||
| .then(response => { | |||
| const stacks = findAndGroupDeployments(response, prefix, service, stage); | |||
| const stacksToKeep = stacks.slice(-stacksToKeepCount || Infinity); | |||
| const stacksToRemove = _.pullAllWith(stacks, stacksToKeep, _.isEqual); | |||
| const stacksToRemove = stacks.filter(i => !stacksToKeep.includes(i)); | |||
There was a problem hiding this comment.
From what I see logic above is about splitting stacks into two slices stacksToKeep and stacksToRemove, and In that case I think stacksToRemove can also be constructed with slice same as stacksToKeep:
stacksToRemove = stacks.slice(0, -stacksToKeepCount || Infinity);I think above notation will be more accurate
There was a problem hiding this comment.
You are right, I focused replacing lodash functions #7747 from this issue. So, I did not see this point. Thanks
There was a problem hiding this comment.
@calganaygun looks good! Can you just ensure branch is up to date with master (so new CI/CD jumps in)
There was a problem hiding this comment.
Thank you @calganaygun !
That way by replacing lodash, we replaced some unnecessary convoluted part with something simple and straightforward achieved just by native means :)
Adresses #7747
_.pullAllWith replaced with Array.filter()