Standardise conventions for comments and naming in Kernel - Githubissues

0xPolygonMiden / miden-base

Core components of the Polygon Miden rollup

MIT License

70 stars 44 forks source link

Standardise conventions for comments and naming in Kernel #858

Open phklive opened 2 months ago

phklive commented 2 months ago

Feature description

Throughout the kernel different conventions for comments and naming have been used, e.g. in account.masm:

https://github.com/0xPolygonMiden/miden-base/blob/7ede403dafc866bd2646ddfb979a76f16d8965af/miden-lib/asm/kernels/transaction/lib/account.masm#L360-L362

https://github.com/0xPolygonMiden/miden-base/blob/7ede403dafc866bd2646ddfb979a76f16d8965af/miden-lib/asm/kernels/transaction/lib/account.masm#L376

https://github.com/0xPolygonMiden/miden-base/blob/7ede403dafc866bd2646ddfb979a76f16d8965af/miden-lib/asm/kernels/transaction/lib/account.masm#L528-L529

https://github.com/0xPolygonMiden/miden-base/blob/7ede403dafc866bd2646ddfb979a76f16d8965af/miden-lib/asm/kernels/transaction/lib/account.masm#L421-L423

This is an isolated example with panics but it can be seen in documentation comments, naming of return values, etc...

Why is this feature needed?

We need to make sure that the code is clean, readable and understandable.

Furthermore setting the right conventions early will enable the project to scale easier.

bobbinth commented 2 months ago

Agreed!

In terms of naming, a few things that I noticed:

get_output_notes_hash should probably be renamed into get_output_notes_commitment.
We should probably use the same convention for all methods in the kernel. For example, we have set_account_item but `account_vault_add_asset. I think it should be either:
- set_account_item and add_account_asset, or
- account_set_item and account_add_asset.