-
Notifications
You must be signed in to change notification settings - Fork 26
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 doc and remove unused constant #183
Conversation
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.
Nit: could you also rename the fr
variables?
// ModuleEntity is a packed representation of a module function | ||
// Layout: | ||
// 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address | ||
// 0x________________________________________BBBBBBBB________________ // Entity ID | ||
// 0x________________________________________________0000000000000000 // unused |
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.
Also maybe newline before this, and consider making this a NatSpec @dev comment?
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.
Oh I see that this is for ModuleEntity
imported above. Is this necessary? IModularAccount already describes it.
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.
It's repeated in both IModularAccount
and each of the type encoding / decoding libraries (ValidationConfigLib
, HookConfigLib
) so I added it here too.
4d3aac3
to
c8b7e28
Compare
Motivation
We have some missing documentation and an unused constant.
Solution
Add the missing comment and remove the constant.