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

chore(deps): pin dependencies #711

Merged
merged 13 commits into from
Dec 17, 2018
Merged

chore(deps): pin dependencies #711

merged 13 commits into from
Dec 17, 2018

Conversation

dhruvdutt
Copy link
Member

What kind of change does this PR introduce?
Dependencies pinning

Did you add tests for your changes?
N/A

If relevant, did you update the documentation?
N/A

Summary

Does this PR introduce a breaking change?
No

Other information
Fixes #697

@webpack-bot
Copy link

@dhruvdutt The most important CI builds failed. This way your PR can't be merged.

Please take a look at the CI results from travis (failure) and fix these issues.

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

This is still in discussion, so this is up for chills

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

Lgtm

@ematipico
Copy link
Contributor

There's a failing test... Does that mean that pinning the dependencies caused a different behaviour of the library? That's odd. We should fix it

@evenstensberg
Copy link
Member

It's from uglify/terser test, which PR changed that again? Was recently done

@ematipico
Copy link
Contributor

ematipico commented Dec 16, 2018

There's an open PR

@evenstensberg
Copy link
Member

If it doesnt fail at that then merging that would be nice

@dhruvdutt dhruvdutt merged commit 62322d5 into webpack:master Dec 17, 2018
@dhruvdutt
Copy link
Member Author

Seems like this is a side effect of something. Pinning dependencies wouldn't change anything.

@dhruvdutt dhruvdutt deleted the deps branch December 17, 2018 06:21
@evenstensberg
Copy link
Member

Let’s do this properly Dhruvdut...

@dhruvdutt
Copy link
Member Author

Some weird issue. It worked locally for me. Looking into it.

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

Successfully merging this pull request may close these issues.

4 participants