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

Make debug loggers throughout monorepo uniform in namespacing #3689

Closed
scorbajio opened this issue Sep 19, 2024 · 2 comments · Fixed by #3692
Closed

Make debug loggers throughout monorepo uniform in namespacing #3689

scorbajio opened this issue Sep 19, 2024 · 2 comments · Fixed by #3692

Comments

@scorbajio
Copy link
Contributor

scorbajio commented Sep 19, 2024

This issue is inspired by incongruities discovered from work done on #3663 and #3676.

The two issues that need addressing include the following:

  1. Debug namespaces are currently named using a mix of casings
  2. Debug namespaces currently don't allow a parent namespace to be included when a wild card is used for including it's subspaces. E.g. DEBUG=ethjs,blockchain:* will include blockchain:clique but not blockchain.

Another issue I want to note is making sure that any additional debug logic is also being guarded with this.DEBUG so to not cause performance slowdowns, such as here.

@jochem-brouwer
Copy link
Member

Could you update this issue to write down what the uniform standard of debug is? I.e. the case casing, and how methods/objects are logged? (Seems like we now have an underscore thing like find_path? And it is all lowercase?)

@scorbajio
Copy link
Contributor Author

scorbajio commented Sep 23, 2024

Could you update this issue to write down what the uniform standard of debug is? I.e. the case casing, and how methods/objects are logged? (Seems like we now have an underscore thing like find_path? And it is all lowercase?)

A brief resource on naming conventions can be found at "Programming Naming Conventions Explained".

I've left some things open-ended for now (will do my best to note), but these are the rules I've implemented so far:

  • Casings should always be lower case.
  • Where necessary, snake case can be used to name either methods or objects or other logable things/events. I found a mix of this in the trie package, for example, where methods like put, get, del, and find_path are tagged using snake case, and objects like extension_node and raw_node as well as even specific conditions such as root values before/after reverts are tagged.
  • The trie package also appends the branch index of nodes looked up in it's onFound callback, so use of numbers is also currently fair-use, but I'd say we can change this in a followup to Debug logger namespace standardization #3692 since it would be more fit to include the branch index in the debug string, unless there are other comments on this.

We can also potentially keep this thread open to evolve our logging standards and naming here.

UPDATE: I've added changes for moving the branch index from the namespace to the debug string in the same PR, #3692.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants