-
Notifications
You must be signed in to change notification settings - Fork 2
refactor: proposal type data struct #401
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: proposal type data struct #401
Conversation
0xOneTony
left a 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.
Good job so far! Lets try to be a bit more careful with natspecs and variable naming both in src and in tests. We want to be as verbose as possible and anyone reading our code should be able to understand at least on a high level what we do.
I am also considering removing the "proposalTypeConfigurator" name all together and rename to "votingModule" but thats for a different PR.
packages/contracts-bedrock/src/governance/ProposalValidator.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/governance/ProposalValidator.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/test/governance/ProposalValidator.t.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/governance/ProposalValidator.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/governance/ProposalValidator.sol
Outdated
Show resolved
Hide resolved
0xOneTony
left a 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.
LGTM! good job
* refactor: use proposalTypesData mapping * docs: improve natspec * feat: add mismatched lenghts check in contract initializer * fix: emit ProposalTypeDataSet with missing arg * refactor: correct naming in test helper functions * refactor: correct variable naming in fuzz tests * perf: remove redundant fields from ProposalData struct * chore: run pre-pr * chore: improve code formatting
* refactor: use proposalTypesData mapping * docs: improve natspec * feat: add mismatched lenghts check in contract initializer * fix: emit ProposalTypeDataSet with missing arg * refactor: correct naming in test helper functions * refactor: correct variable naming in fuzz tests * perf: remove redundant fields from ProposalData struct * chore: run pre-pr * chore: improve code formatting
Closes OPT-800, OPT-807
🐧 ?