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

cpp: Rename evmc::vm to evmc::VM #252

Merged
merged 2 commits into from
Sep 19, 2019
Merged

cpp: Rename evmc::vm to evmc::VM #252

merged 2 commits into from
Sep 19, 2019

Conversation

chfast
Copy link
Member

@chfast chfast commented Apr 24, 2019 •
edited
Loading

Closes #249

@axic
Copy link
Member

axic commented Apr 24, 2019

Was this introduced in this release or is this a breaking change?

The rest of the C++ interfaces are camel cased so this is consistent with it, with one exception: result. Would changing that to Result be breaking?

@chfast
Copy link
Member Author

chfast commented Apr 24, 2019

Nothing strict here, but have a feeling that types representing OOP classes should have camel case names. The result was left because it's more like unique_ptr but can be renamed as well.

The vm class was added in 6.2. I have to check the result.

@chfast
Copy link
Member Author

chfast commented Apr 24, 2019

Both were added in 6.1: https://github.com/ethereum/evmc/blob/v6.1.1/include/evmc/evmc.hpp.

@chfast chfast closed this Apr 24, 2019
@axic
Copy link
Member

axic commented Aug 21, 2019 •
edited
Loading

@chfast reviving this? (Or deleting branch?)

@chfast chfast reopened this Aug 22, 2019
@chfast chfast requested a review from axic August 22, 2019 07:53
@axic
Copy link
Member

axic commented Aug 22, 2019

@chfast will Result handled in a separate PR?

@chfast
Copy link
Member Author

chfast commented Sep 19, 2019

@chfast will Result handled in a separate PR?

I'm not sure about result. It is rather a value type (like string).

@chfast chfast added the changelog Deserves a CHANGELOG entry label Sep 19, 2019
@chfast chfast force-pushed the cpp-rename-vm branch 5 times, most recently from 49f23f2 to 57082ec Compare September 19, 2019 15:22
@chfast
cpp: Rename evmc::vm to evmc::VM
0338ac4
@chfast
cpp: Move VM construction implementation out of class definition
40fdf96 
This workarounds a bug in doxygen which incorrectly parses the implementation with for range loop.
@chfast chfast merged commit 6123b66 into master Sep 19, 2019
@chfast chfast deleted the cpp-rename-vm branch September 19, 2019 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axic axic axic approved these changes

Assignees
No one assigned
Labels
changelog Deserves a CHANGELOG entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

C++: Rename evmc::vm to evmc::VM
2 participants
@chfast @axic