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

Add eth.getChainId method to 1.x #3113

Merged
merged 3 commits into from
Oct 8, 2019
Merged

Add eth.getChainId method to 1.x #3113

merged 3 commits into from
Oct 8, 2019

Conversation

gabmontes
Copy link
Contributor

@gabmontes gabmontes commented Oct 8, 2019

This method was added to the branch 2.x in d98cef2. This change is to support the method in the 1.x line.

Worth considering this PR in the discussion at #3070 .

@gabmontes gabmontes changed the title Add eth.getChainId method Add eth.getChainId method to 1.x Oct 8, 2019
@coveralls
Copy link

coveralls commented Oct 8, 2019

Coverage Status

Coverage remained the same at 83.067% when pulling 9f6c813 on bloq:add-getchainid into a790dd6 on ethereum:1.x.

@nivida nivida added 1.x 1.0 related issues Feature Request labels Oct 8, 2019
Copy link
Contributor

@nivida nivida 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 to me. Thanks! :)

@nivida nivida requested a review from cgewecke October 8, 2019 21:39
Copy link
Collaborator

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

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

LGTM as well.

NB: eth.getChainId needs to be added to the documentation at some point.

@nivida
Copy link
Contributor

nivida commented Oct 8, 2019

@cgewecke true that. It can be copied from the 2.x branch.

@gabmontes Do you have the time to do this for us?

This method was added to the branch 2.x in d98cef2. This change is to support the method in the 1.x line.
@gabmontes
Copy link
Contributor Author

@nivida added docs and rebased to get rid of the merge conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants