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

[Refactor] Nit: align naming of tx_hash / transaction_hash, tx_type / transaction_type #814

Closed
zerosnacks opened this issue Jun 3, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@zerosnacks
Copy link
Member

zerosnacks commented Jun 3, 2024

Component

consensus, eips, genesis, network, json-rpc, rpc

Describe the feature you would like

A nit, but I noticed an inconsistency when switching between a pending transaction (.tx_hash()) and receipt (.transaction_hash):

For Log and Receipt: transaction_hash is used.
For TxEnvelope, Heartbeat, TraceResult: tx_hash is used.

Another case is tx_type and transaction_type:

For Transaction, TransactionReceipt, TransactionRequest: transaction_type is used.
For ReceiptEnvelope and : tx_type is used.

I propose we align the naming. My preference is transaction_hash and transaction_type, this is in line with signature_hash.

Additional context

No response

@zerosnacks zerosnacks added the enhancement New feature or request label Jun 3, 2024
@DaniPopes DaniPopes added this to the 0.1-rc.0 milestone Jun 5, 2024
@DaniPopes DaniPopes removed the 0.1-rc.0 label Jun 5, 2024
@zerosnacks zerosnacks self-assigned this Jun 5, 2024
@gakonst
Copy link
Member

gakonst commented Jun 5, 2024

It seems like the main motivator here is intellisense so if we do something we could use doc(alias) for things that make sense for autocomplete, and keep the other type names the same.

@gakonst gakonst removed this from the 0.1-rc.0 milestone Jun 5, 2024
@zerosnacks zerosnacks removed their assignment Jun 6, 2024
@mattsse mattsse closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants