-
Notifications
You must be signed in to change notification settings - Fork 310
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: update solhint #9399
fix: update solhint #9399
Conversation
e5a401d
to
4393c3f
Compare
ba54ec6
to
ff76321
Compare
4393c3f
to
4eefa0b
Compare
ff76321
to
2863f0d
Compare
2863f0d
to
3f07135
Compare
} | ||
} | ||
|
||
using SlotLib for Slot; | ||
using EpochLib for Epoch; | ||
|
||
function addTimestamp(Timestamp a, Timestamp b) pure returns (Timestamp) { | ||
return Timestamp.wrap(Timestamp.unwrap(a) + Timestamp.unwrap(b)); | ||
function _addTimestamp(Timestamp _a, Timestamp _b) pure returns (Timestamp) { |
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.
Not sure a rule where free functions need to be _ makes sense? They will be used in other files so I dont see them as "private"
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.
Ye, taking a look at altering the rule to see how simple it is, hopefully straight forward.
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.
} | ||
|
||
function executableThrough(DataStructures.Proposal storage self) | ||
function executableThrough(DataStructures.Proposal storage _self) |
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.
storage values getting _ also seems odd to me, maybe a new prefix? - $?
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.
Still a param here, but I get you. Think it is easier for us to deal with it later though as it would need some extra rules for general storage as well if we want to be more consistant I guess 🤔
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.
Have nits but thanks for getting it done
3f07135
to
db70936
Compare
Changes to public function bytecode sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
* master: (83 commits) fix: update solhint (#9399) chore: redo typo PR by leopardracer (#9705) feat: Encode static error strings in the ABI (#9552) chore: redo typo PR by donatik27 (#9693) chore: update install instructions for foundry to display version of rust to install (#9653) chore: disable bench upload until #9692 fix: earthly-ci in bench-e2e (#9689) chore: redo typo PR by cypherpepe (#9687) chore: redo typo PR by youyyytrok (#9686) chore: redo typo PR by mdqst (#9685) chore: redo typo PR by mdqst (#9684) feat: adding tags to encrypted logs (#9566) fix: enable gerousia e2e test (#9677) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg chore: redo typo PR by dsarfed (#9667) fix: bench e2e jobs (#9662) fix: Fix random for Mac users (#9670) ...
Fixes #8943
Updated solhint in https://github.com/LHerskind/solhint to get the newest things from the public version and then applied some extra rules for our setup and update the docker and earthfile such that we actually use that version of solhint.