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

Rename getBalance to getExternalBalance #120

Merged
merged 1 commit into from
Aug 16, 2018
Merged

Rename getBalance to getExternalBalance #120

merged 1 commit into from
Aug 16, 2018

Conversation

axic
Copy link
Member

@axic axic commented Jul 31, 2018

It loads an external account and as such this naming reflects it better.

@jakelang
Copy link
Member

jakelang commented Aug 1, 2018

Perhaps getBalanceAtAddress may make more sense upon initial reading?

@axic
Copy link
Member Author

axic commented Aug 1, 2018

Good idea, although I'd suggest that a proposal is created which renames all the getExternal<xyz> to a scheme get<xyz>AtAddress

@chfast
Copy link
Collaborator

chfast commented Aug 1, 2018

There is a naming convention in RPC that applied here would mix both proposals to getBalanceAt(), but in English it would be better to have getBalanceOf().

@jakelang
Copy link
Member

jakelang commented Aug 3, 2018

@axic would changing the naming scheme make sense if we want to keep it similar to EVM opcode names?

Or perhaps should we change such names in a later revision where we depart from EVM compatibility?

@chfast
Copy link
Collaborator

chfast commented Aug 8, 2018

@jakelang I think "similar" is good enough. It should not be a problem for match mentally BALANCE opcode with getExternalBalance or getBalanceOf.

@axic axic merged commit 5862c9f into master Aug 16, 2018
@axic axic deleted the getexternalbalance branch August 16, 2018 16:47
@axic axic removed the in review label Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants