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

go: Remove dependencies on go-ethereum #513

Merged
merged 1 commit into from
May 14, 2020
Merged

go: Remove dependencies on go-ethereum #513

merged 1 commit into from
May 14, 2020

Conversation

chfast
Copy link
Member

@chfast chfast commented May 14, 2020 •
edited
Loading

Continuation of #499.

I added a change not to define {address,hash}Length constants. To me this looks fine.
Because of the change I would just move the Address and Hash types to evmc.go.

@chfast chfast mentioned this pull request May 14, 2020
Closed
@chfast chfast requested a review from axic May 14, 2020 09:42
axic
axic reviewed May 14, 2020
View reviewed changes
bindings/go/evmc/types.go Outdated Show resolved Hide resolved
@axic
Copy link
Member

axic commented May 14, 2020 •
edited
Loading

@mandrigin @chfast Should Error (https://github.com/ethereum/evmc/blob/remove-geth-types/bindings/go/evmc/evmc.go#L71) be also moved to types?

Also evmc.go has Revision and Capability, while host.go has CallKind, StorageStatus, TxContext.

Any reason for this layout? Maybe not worth addressing these in this PR, but could be kept in mind for the future.

Lastly the commit message has a typo "Lenght" vs "Length".

@chfast chfast marked this pull request as ready for review May 14, 2020 10:36
mandrigin
mandrigin approved these changes May 14, 2020
View reviewed changes
Copy link
Collaborator

@mandrigin mandrigin left a comment

Choose a reason for hiding this comment

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

looks good!

@chfast
Copy link
Member Author

chfast commented May 14, 2020

Should Error (https://github.com/ethereum/evmc/blob/remove-geth-types/bindings/go/evmc/evmc.go#L71) be also moved to types?

Because of the change I would just move the Address and Hash types to evmc.go.

@mandrigin @chfast
go: Remove dependencies on go-ethereum
796905b 
This introduces evmc.Address and evmc.Hash types as a replacement of
using ones from go-ethereum/common.
@chfast chfast merged commit 2cba641 into master May 14, 2020
@chfast chfast deleted the remove-geth-types branch May 14, 2020 12:50
@axic
Copy link
Member

axic commented May 14, 2020

Argh just noticed, couldn't these things be removed from CI: https://github.com/ethereum/evmc/pull/486/files#diff-180360612c6b8c4ed830919bbb4dd459L41

@chfast
Copy link
Member Author

chfast commented May 14, 2020

What do you mean?

@axic
Copy link
Member

axic commented May 14, 2020

Never mind, this PR did remove those lines.

@chfast chfast added the go label May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mandrigin mandrigin mandrigin approved these changes

@axic axic axic approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@chfast @axic @mandrigin