Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

lodash - move to npm, upgrade to lodash 4 #4578

Merged
merged 8 commits into from
Sep 3, 2018
Merged

lodash - move to npm, upgrade to lodash 4 #4578

merged 8 commits into from
Sep 3, 2018

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Aug 31, 2018

This moves all lodash to npm - that means we no longer have lodash 3 available anywhere, and are always using lodash 4.

Following https://github.com/lodash/lodash/wiki/Changelog#compatibility-warnings to fix any incompatibilities.

Issue #3734

@miq-bot miq-bot changed the title lodash - move to npm, upgrade to lodash4 [WIP] lodash - move to npm, upgrade to lodash4 Aug 31, 2018
@miq-bot miq-bot added the wip label Aug 31, 2018
@himdel himdel changed the title [WIP] lodash - move to npm, upgrade to lodash4 [WIP] lodash - move to npm, upgrade to lodash 4 Aug 31, 2018
this moves all lodash to npm

that means we no longer have lodash 3 available anywhere
we already had a bunch of comments re lodash4 compatibility, fixing those
and given the one used is _.isEqual, we don't really need `differenceBy`
except we have all the shims, so using JS-native includes
@miq-bot
Copy link
Member

miq-bot commented Sep 3, 2018

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/7ac0a2a42f4d2376954370ce307ad6760729af0f~...30383e3da9b487cdebc57e1cd531c4b3bac87d47 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. ⭐

@himdel
Copy link
Contributor Author

himdel commented Sep 3, 2018

Also verified ui-components should no longer depend on lodash3 at all.

@himdel
Copy link
Contributor Author

himdel commented Sep 3, 2018

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] lodash - move to npm, upgrade to lodash 4 lodash - move to npm, upgrade to lodash 4 Sep 3, 2018
@miq-bot miq-bot removed the wip label Sep 3, 2018
var seriesIndex = _.contains(['Pie', 'Donut'], generate_args.miqChart) ? index : index - 1;
var pointIndex = _.contains(['Pie', 'Donut'], generate_args.miqChart) ? index : data.index;
var seriesIndex = ['Pie', 'Donut'].includes(generate_args.miqChart) ? index : index - 1;
var pointIndex = ['Pie', 'Donut'].includes(generate_args.miqChart) ? index : data.index;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope we got a shim for this...

Copy link
Contributor Author

@himdel himdel Sep 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

es6-shim :) EDIT: or array-includes maybe :)

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Seal of Approval

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants