Skip to content
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

Upgrade UglifyJS to address issues #16

Merged
merged 1 commit into from
Apr 26, 2017

Conversation

robbytx
Copy link
Contributor

@robbytx robbytx commented Apr 24, 2017

@sirugh
Copy link
Contributor

sirugh commented Apr 24, 2017

Isn't uglifyjs2 the NEWER version of uglifyjs? This is a scary change.

@tnunamak
Copy link

tnunamak commented Apr 24, 2017

Yes, but it appears that the uglify-js package is actually now UglifyJS2 if you install a 2.x version (and actually, there's a version 3 now, but still in the UglifyJS2 project, if you want to be even more confused).

@robbytx
Copy link
Contributor Author

robbytx commented Apr 24, 2017

@sirugh npm install warns:

npm WARN deprecated uglify-js2@2.1.11: You can now install this using uglify-js@2.2.0 node_modules/uglify-js

So, uglify-js@2.2.0 (and beyond) actually supercede uglify-js2@2.1.11

@robbytx
Copy link
Contributor Author

robbytx commented Apr 24, 2017

Do I need to upgrade package.json in my commit?

From a semver perspective, would you consider this a patch release? I'm not sure what all changed in uglify-js from 2.1.11 to 2.8.22, and whether those changes would indicate this change is a minor release.

@sirugh
Copy link
Contributor

sirugh commented Apr 24, 2017

@robbytx This is a patch, but we will handle that in the "release".

@aterranova-bv aterranova-bv merged commit c112ab3 into bazaarvoice:master Apr 26, 2017
@robbytx
Copy link
Contributor Author

robbytx commented Apr 26, 2017

@KikoRamos has verified this in BV Conversations against supported browsers, and I've verified this in our in-development application.

@robbytx robbytx deleted the upgrade-uglify-js branch April 26, 2017 17:45
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.

None yet

4 participants