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

fix(bindgen): don't rename to index #105

Merged
merged 1 commit into from
Apr 23, 2018
Merged

fix(bindgen): don't rename to index #105

merged 1 commit into from
Apr 23, 2018

Conversation

ashleygwilliams
Copy link
Member

fixes #102

@alexcrichton
Copy link
Contributor

FWIW if index.js is desired then it's actually relatively easy to rewrite imports of wasm modules (especially with parity-wasm), so that's at least an other option here although certainly not required if this strategy also works!

mgattozzi
mgattozzi approved these changes Apr 23, 2018
View reviewed changes
Copy link
Contributor

@mgattozzi mgattozzi left a comment

Choose a reason for hiding this comment

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

LGTM but does this mean we need to reopen #94?

@ashleygwilliams
Copy link
Member Author

@mgattozzi it doesn't we should be all set by declaring it as main. @alexcrichton good to know, i really wanna cut a 0.2.0 today so i'll fix this now but look into rewriting the import and also making sure we have tests so we dont have a regression like this

@mgattozzi
Copy link
Contributor

Perfect! Then this definitely LGTM. Should we open a separate issue to address @alexcrichton's stuff later on when we have more time?

@ashleygwilliams ashleygwilliams merged commit a906fe6 into master Apr 23, 2018
@ashleygwilliams ashleygwilliams deleted the woops branch April 23, 2018 20:31
@ashleygwilliams ashleygwilliams mentioned this pull request Apr 23, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgattozzi mgattozzi mgattozzi approved these changes

Assignees
No one assigned
Labels
needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

don't rename the js file
3 participants
@ashleygwilliams @alexcrichton @mgattozzi