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.

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

go: Add missing Shanghai revision constant #608

Merged
merged 3 commits into from
Jun 30, 2021
Merged

go: Add missing Shanghai revision constant #608

merged 3 commits into from
Jun 30, 2021

Conversation

chfast
Copy link
Collaborator

@chfast chfast commented Jun 29, 2021

Added a test that fails in case of C.EVMC_MAX_REVISION is increased as a remainder to add new Revision constants to Go.

evmc_test.go:65: missing constant for revision 10

The missing London Revision is added.

@chfast chfast requested review from axic and gumb0 June 29, 2021 07:29
@codecov
Copy link

codecov bot commented Jun 29, 2021

Codecov Report

Merging #608 (4678a21) into master (6b05379) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #608   +/-   ##
=======================================
  Coverage   95.71%   95.71%           
=======================================
  Files          24       24           
  Lines        3971     3971           
=======================================
  Hits         3801     3801           
  Misses        170      170           

@@ -149,6 +149,8 @@ const (
Istanbul Revision = C.EVMC_ISTANBUL
Berlin Revision = C.EVMC_BERLIN
London Revision = C.EVMC_LONDON
Shanghai Revision = C.EVMC_SHANGHAI
maxRevision Revision = C.EVMC_MAX_REVISION
Copy link
Member

Choose a reason for hiding this comment

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

Why not also add latest stable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maxRevision is private and cannot be used outside of the evmc package.

Copy link
Member

Choose a reason for hiding this comment

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

How is it private? Because lowercase?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. But made it public now together with LatestStableRevision. I don't want to overthink it.

@@ -149,6 +149,8 @@ const (
Istanbul Revision = C.EVMC_ISTANBUL
Berlin Revision = C.EVMC_BERLIN
London Revision = C.EVMC_LONDON
Shanghai Revision = C.EVMC_SHANGHAI
maxRevision Revision = C.EVMC_MAX_REVISION
)

Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, but can't comment on the line: can func (err Error) Error() string be replaced by the new string helpers?

@@ -59,3 +59,9 @@ func TestExecuteEmptyCode(t *testing.T) {
t.Errorf("execution returned unexpected error: %v", err)
}
}

func TestRevision(t *testing.T) {
if MaxRevision != London {
Copy link
Member

Choose a reason for hiding this comment

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

Should there also be a test for LatestStableRevision here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added.

@chfast chfast merged commit 3bb50e2 into master Jun 30, 2021
@chfast chfast deleted the go_shanghai_rev branch June 30, 2021 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants