-
Notifications
You must be signed in to change notification settings - Fork 70
[Fix] Add names for UtreexoNode fields #312
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
[Fix] Add names for UtreexoNode fields #312
Conversation
|
I like the idea. Just don't really like |
f6d4c1f to
d7d5f6f
Compare
|
I dont like context so much... i had to spend 2 entire hours reading the code trying to understand what context is... Since this can be a skill issue of me i implemented the changes and you can merge at it is or i can change again if you decide to. |
d7d5f6f to
35837dc
Compare
|
Forgot the generic type declaration. |
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.
LGTM. Just one small thing
35837dc to
7f77d16
Compare
|
Changed the generic name on UtreexoNode |
7f77d16 to
3c72c6c
Compare
|
Rebased |
|
There's a config with |
|
Yes, i was waiting for #310 to get merged |
…osition of the field
3c72c6c to
f2e2e36
Compare
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.
ACK f2e2e36
While searching a way to implement
MTP, i got myself a lot of the times with theUtreexoNodedeclaration aside my readings trough florestad`s internals just becauseThe generic trait declaration is inverted and the fields had no name, usually being called as
self.0orself.1which is not so intuitive to look at.IMO is better to always name internal fields if they're being accessed from outside the struct and not only from internal methods for that structure