Skip to content

include full build of Lodash#1967

Merged
Be-ing merged 1 commit intomixxxdj:masterfrom
Be-ing:lodash_full_build
Jan 5, 2019
Merged

include full build of Lodash#1967
Be-ing merged 1 commit intomixxxdj:masterfrom
Be-ing:lodash_full_build

Conversation

@Be-ing
Copy link
Copy Markdown
Contributor

@Be-ing Be-ing commented Dec 28, 2018

Some mapping developers have asked for this and I don't think there's a good reason not to include all of Lodash. We are not optimizing every kilobyte of download size for a website in this context.

Also disable strict mode, which somehow breaks with QJSEngine (#1795) and Qt 5.12.

Also disable strict mode, which somehow breaks with QJSEngine
and Qt 5.12.
@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Dec 28, 2018

I searched the Qt bug tracker for new issues with 'use strict'; in 5.12 but didn't find anything relevant. I'm not sure if the problem is in Lodash or Qt, but simply not using strict mode works 🤷‍♂️

@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Dec 28, 2018

Actually, I'm thinking it would be better to remove the dependency on Lodash and use plain ES7...

@Be-ing Be-ing closed this Dec 28, 2018
@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Dec 28, 2018

Superceded by ferranlala#6

@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Dec 30, 2018

Following discussion on Zulip, I changed my mind. I think we should merge this to keep compatibility with Qt 5.9 in #1795 for Ubuntu 18.04 LTS.

@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Jan 2, 2019

Any objections or should we merge this?

@uklotzde
Copy link
Copy Markdown
Contributor

uklotzde commented Jan 5, 2019

No objections. Including the whole, unmodified library should reduce the risk to confuse mapping developers. Ready to merge?

@Be-ing Be-ing merged commit 15371b0 into mixxxdj:master Jan 5, 2019
@Be-ing Be-ing deleted the lodash_full_build branch April 24, 2019 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants