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

Update example to rust 2018 #30

Merged
merged 20 commits into from
Sep 6, 2019
Merged

Update example to rust 2018 #30

merged 20 commits into from
Sep 6, 2019

Conversation

theJian
Copy link

@theJian theJian commented Mar 23, 2019 •
edited by amilajack
Loading

Fix #23

Note: we'll also need to update the docs to reflect these changes

@theJian
Copy link
Author

theJian commented Mar 23, 2019

Electron example doesn't run. I'm not sure if it's because it depends on neon-hello that depends on an early version of neon.

@theJian
Copy link
Author

theJian commented Mar 23, 2019

/examples/electron-app/node_modules/neon-hello/native/index.node: undefined symbol: __cxa_pure_virtual

This error will raise when I run npm start under electron example folder.

@theJian
Copy link
Author

theJian commented Mar 23, 2019

I think we can replace neon-hello with hello-world example so the electron example can use local code without needing to download additional dependency. What do you think @amilajack

@theJian theJian mentioned this pull request Mar 23, 2019
Closed
@amilajack
Copy link
Member

@theJian sounds good!

@theJian theJian changed the title [WIP]: Update example to rust 2018 Update example to rust 2018 Mar 31, 2019
@theJian
Copy link
Author

theJian commented Mar 31, 2019

Electron example doesn't work on my Linux machine.
neon-bindings/neon#394 is existing for this issue.

I'll do more investigation. If I come up with any solution I'll open another pr.

@amilajack
Copy link
Member

Awesome! Can you rustfmt on the changes if you haven't already? Thanks!

@theJian
Copy link
Author

theJian commented Apr 1, 2019

Awesome! Can you rustfmt on the changes if you haven't already? Thanks!

Done. I have run rustfmt on every example.

@amilajack
Copy link
Member

@theJian sorry for the delay on this. If you have time can you please fix the merge conflicts. I'll merge this right after that

@theJian
Copy link
Author

theJian commented Sep 4, 2019

Conflicts resolved!

kjvalencik
kjvalencik reviewed Sep 4, 2019
View reviewed changes
errors/native/src/lib.rs Outdated Show resolved Hide resolved
kjvalencik
kjvalencik reviewed Sep 4, 2019
View reviewed changes
errors/native/src/lib.rs Outdated Show resolved Hide resolved
amilajack
amilajack requested changes Sep 4, 2019
View reviewed changes
Copy link
Member

@amilajack amilajack left a comment

Choose a reason for hiding this comment

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

First of all, thanks for the PR! We should be able to merge this after you address KJ's review feedback

@amilajack amilajack merged commit 931a624 into neon-bindings:master Sep 6, 2019
@theJian theJian deleted the issue-23 branch September 6, 2019 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amilajack amilajack amilajack approved these changes

@kjvalencik kjvalencik kjvalencik approved these changes

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

Successfully merging this pull request may close these issues.

update examples to rust 2018
3 participants
@theJian @amilajack @kjvalencik