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

Remove support for C++/Javascript version of evm2wasm #417

Merged
merged 5 commits into from
Nov 26, 2018
Merged

Remove support for C++/Javascript version of evm2wasm #417

merged 5 commits into from
Nov 26, 2018

Conversation

axic
Copy link
Member

@axic axic commented Sep 14, 2018

No description provided.

@axic
Copy link
Member Author

axic commented Nov 8, 2018

@chfast after fixing this, can we merge this for 0.2.1?

@chfast
Copy link
Collaborator

chfast commented Nov 8, 2018

@chfast after fixing this, can we merge this for 0.2.1?

Yes, why not.

@chfast
Copy link
Collaborator

chfast commented Nov 10, 2018

Merge #453 and rebase this one.

@axic
Copy link
Member Author

axic commented Nov 11, 2018

Either depends on #375 or all evm2wasm tests need to be removed.

@chfast
Copy link
Collaborator

chfast commented Nov 12, 2018

I'm for disabling evm2wasm tests and re-enable them when runevm is ready.

@codecov-io
Copy link

codecov-io commented Nov 24, 2018 •
edited
Loading

Codecov Report

Merging #417 into master will increase coverage by 2.87%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #417      +/-   ##
==========================================
+ Coverage   69.29%   72.16%   +2.87%     
==========================================
  Files           8        8              
  Lines        1104     1060      -44     
  Branches      137      132       -5     
==========================================
  Hits          765      765              
+ Misses        308      264      -44     
  Partials       31       31

@axic
Copy link
Member Author

axic commented Nov 24, 2018

Build is failing due to ethereum/evmc#172

@axic
Copy link
Member Author

axic commented Nov 26, 2018

@chfast even if you close ethereum/evmc#172 this is still broken and needs to be addressed :)

@chfast
Copy link
Collaborator

chfast commented Nov 26, 2018

@chfast even if you close ethereum/evmc#172 this is still broken and needs to be addressed :)

No kidding.

axic
axic commented Nov 26, 2018
View reviewed changes
CMakeLists.txt
@@ -12,18 +12,16 @@ include(CableToolchains)
include(CMakePackageConfigHelpers)
include(GNUInstallDirs)

option(EVMC_TESTING "Build EVMC tests and test tools" OFF)
Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't this a foreign key? ${EVMC_TESTING} instead of OFF?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No. It's defined before EVMC defines it. I think this line can be dropped actually.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll just merge this now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will remove the line.

@axic axic changed the title [WIP] Remove support for C++/Javascript version of evm2wasm Remove support for C++/Javascript version of evm2wasm Nov 26, 2018
@axic axic merged commit 080abfa into master Nov 26, 2018
@axic axic deleted the remove-evm2wasm branch November 26, 2018 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chfast chfast chfast left review comments

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

Successfully merging this pull request may close these issues.
3 participants
@axic @chfast @codecov-io