search

paradigmxyz / reth

Modular, contributor-friendly and blazing-fast implementation of the Ethereum protocol, in Rust
https://reth.rs/
Apache License 2.0
3.86k stars 1.11k forks source link

Improve constant names #6464

Closed emhane closed 3 months ago

emhane commented 8 months ago

Describe the feature

To improve maintainability, constant names need to be revised. Constants names need to be more descriptive and words in constant names need to follow a grammar: position (1) what is the constant ultimately? Is it a DEFAULT, MAX, MIN, AVERAGE, MEDIAN, TEST, SOFT_LIMIT etc.? position (2) may come a word from (1), like we could have a DEFAULT_MAX_..., otherwise (3) position (3) what's the unit? Is it BYTES, COUNT, LENGTH, RETRIES (retries is a specialised form of count), etc. position(4) what is the thing that we are measuring? For example GET_POOLED_TRANSACTIONS_REQUEST, GET_POOLED_TRANSACTIONS_REQUEST_BODY, etc. If that thing refers to specific type, please include the whole type's name. position (5) optionally we may need to describe (4) based on the context it's found in, e.g. INBOUND, OUTBOUND, AS_PENDING, POST_IMPORT, etc.

An example is DEFAULT_MAX_COUNT_HASHES_IN_GET_POOLED_TRANSACTIONS_REQUEST. Skipping the preposition is fine too if it remains unambiguous what the constant value holds DEFAULT_MAX_COUNT_HASHES_GET_POOLED_TRANSACTIONS_REQUEST.

That the constant name is long, is not a problem. If it isn't descriptive enough, it may result in some tedious bugs caused by misinterpretation of the constant from interpreting it based on the context it's found in, as compensation for the lack of descriptiveness in the name.

Additional context

No response

JP2308 commented 8 months ago

Happy to make a start on this as I become more familiar with the codebase!

emhane commented 7 months ago

Happy to make a start on this as I become more familiar with the codebase!

probably makes sense to do this in parts if you're still keen @JP2308 since otherwise you may find yourself endlessly chasing main branch! maybe crate by crate?

shakeib98 commented 7 months ago

Is there any update on this? If not I can take this one. I can do crate by crate, rather than chasing main branch.

JP2308 commented 7 months ago

I have found myself chasing the main a bit.

I am making my way though it but slowly given my inexperience. I understand if you'd like others to take this on given they can work faster.

emhane commented 7 months ago

how about writing here which crate your are currently on @JP2308 and @shakeib98 so you're not working on the same one?

shakeib98 commented 7 months ago

Yeah that makes sense. @JP2308 , please mention crates or folders you are taking on, I'll take other then.

JP2308 commented 7 months ago

Sounds like a plan, I have completed the following crates:

Maybe @shakeib98 you start from the bottom and we meet somewhere in the middle?

shakeib98 commented 7 months ago

Which crate is etl?

Also, I will take the following crates then

  1. trie
  2. transaction-pool
  3. tracing
  4. tokio-util
  5. task
  6. storage
  7. stages
  8. snapshot
  9. rpc
  10. revm
  11. prune
  12. primitives
  13. payload

In total there are 25 crates at the moment, you can take the upper 12 I'll complete the rest 13.

github-actions[bot] commented 6 months ago

This issue is stale because it has been open for 21 days with no activity.