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.

Sign up for GitHub

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

Jump to bottom

Add es6-promise polyfill to webpack config #1765

Merged
merged 1 commit into from
Sep 22, 2016
Merged

Add es6-promise polyfill to webpack config #1765

merged 1 commit into from
Sep 22, 2016

Conversation

gnestor
Copy link
Contributor

@gnestor gnestor commented Sep 14, 2016

Implements #1754 (comment)

@gnestor gnestor added this to the 5.0 milestone Sep 14, 2016
@gnestor
Copy link
Contributor Author

gnestor commented Sep 14, 2016

@blink1073 I see that moment and underscore are listed as both npm and bower dependencies so I figure that they can be removed from bower. However, removing them causes conflicts... How do I resolve conflicts when changing dependencies?

@blink1073
Copy link
Contributor

What do you mean by conflicts, merge conflicts? That should only happen if you weren't basing your PR off of master.

@gnestor
Copy link
Contributor Author

gnestor commented Sep 14, 2016

I'm actually not sure. You can see that "All checks have failed" and I'm seeing "This branch has conflicts that must be resolved". I created this branch from master, so I'm assuming that this is related to me changing the npm/bower dependencies?

@blink1073
Copy link
Contributor

What I mean is that you probably did not have the latest commit from jupyter/notebook master as the basis of your PR, you probably need to fetch and rebase.

@gnestor gnestor force-pushed the webpack-es6-promise branch from 495c5ae to 222b4db Compare September 14, 2016 17:46
@gnestor
Copy link
Contributor Author

gnestor commented Sep 14, 2016

You're right, something weird happened...

@gnestor gnestor changed the title Add es6-polyfill to webpack config Add es6-promise polyfill to webpack config Sep 16, 2016
@gnestor
Copy link
Contributor Author

gnestor commented Sep 20, 2016

Travis is complaining that The command "pip install -f travis-wheels/wheelhouse file://$PWD#egg=notebook[test]" failed and exited with 1 during . for every build. What does that mean?

@takluyver
Copy link
Member

I think the significant error message is this bit:

    AssertionError: No files match pattern notebook/static/components/es6-promise/*.js

Some files are not landing where the Python packaging machinery expects. The package_data is specified here:

notebook/setupbase.py

Line 133 in 179bb24

static_data.extend([

takluyver
takluyver reviewed Sep 21, 2016
View reviewed changes
setupbase.py
@@ -135,7 +135,6 @@ def find_package_data():
pjoin(components, "bootstrap", "js", "bootstrap.min.js"),
pjoin(components, "bootstrap-tour", "build", "css", "bootstrap-tour.min.css"),
pjoin(components, "bootstrap-tour", "build", "js", "bootstrap-tour.min.js"),
pjoin(components, "es6-promise", "*.js"),
Copy link
Member

Choose a reason for hiding this comment

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

I don't know exactly where npm puts these files, but I suspect they should still be included in the tarball somehow...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just pushed a new commit that provides the npm paths to these libs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still failing and I think it's because it's looking for these files before npm install...

@gnestor gnestor force-pushed the webpack-es6-promise branch from 7dfbc68 to 9a4eb25 Compare September 21, 2016 17:03
@gnestor
Copy link
Contributor Author

gnestor commented Sep 21, 2016

How can I resolve this bower.json conflict?

@takluyver
Copy link
Member

It must have changed in master as well - fetch master from github, and then git rebase master to start rebasing. This is a bit fiddly if you haven't done it before, but you can probably find info online about it.

@takluyver
Copy link
Member

Oh, and once you've rebased you'll need to force push (git push -f) to update the branch on Github.

@gnestor gnestor force-pushed the webpack-es6-promise branch 3 times, most recently from dcf4cf0 to 65d9c28 Compare September 21, 2016 19:17
@gnestor
Copy link
Contributor Author

gnestor commented Sep 21, 2016

Ok I'm just going to forget about removing duplicate dependencies and maybe create an issue about it...

@gnestor gnestor force-pushed the webpack-es6-promise branch from 9bcd580 to 7e3e9f3 Compare September 21, 2016 20:19
@gnestor gnestor mentioned this pull request Sep 21, 2016
Closed
@gnestor
Copy link
Contributor Author

gnestor commented Sep 21, 2016

@blink1073 I think this is ready to be merged...

@blink1073
Copy link
Contributor

👍, I don't have push rights in this repo.

@gnestor
Copy link
Contributor Author

gnestor commented Sep 22, 2016

@takluyver I think this ready to be merged!

@takluyver takluyver merged commit bb3b2d5 into jupyter:master Sep 22, 2016
@gnestor gnestor deleted the webpack-es6-promise branch September 22, 2016 15:02
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@takluyver takluyver takluyver left review comments

Assignees
No one assigned
Labels
bug status:resolved-locked tag:Environment
Projects
None yet
Milestone
5.0
Development

Successfully merging this pull request may close these issues.
3 participants
@gnestor @blink1073 @takluyver