-
Notifications
You must be signed in to change notification settings - Fork 82
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
Chore/get paused precompiles name #609
base: develop
Are you sure you want to change the base?
Conversation
Note that this goes against Rust API guidelines https://rust-lang.github.io/api-guidelines/naming.html#getter-names-follow-rust-convention-c-getter |
Why didn't CI run on this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this naming convention much more. @joshuajbouw, however, IMO, the even better naming would be get_paused_precompiles_flags()
because it actually returns flags. But the current naming falsely supposes that the function returns a list of paused precompiles (names, indices, whatever).
@RomanHodulak NB from the same guidelines: The get naming is used only when there is a single and obvious thing that could reasonably be gotten by a getter.
. I think this is the cause.
@sept-en Yes, I typically use "get" if there actually is a storage get myself. It describes that it actually is getting something and costing some computational resources opposed to grabbing something stored in the type itself. |
Description
Simply renames
paused_precompiles
toget_paused_precompiles
to fit in line with the rest of our view naming conventions.A consideration is that this is only breaking for those using the latest commit of the
develop
branch, but not a breaking change for the deployed binary. With that in mind, it is safe to change it now.Performance / NEAR gas cost considerations
None.
Testing
A test was updated to use
get_paused_precompiles
instead.How should this be reviewed
The CI should catch any issues, no special requirements needed to review.
Additional information
None.