-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
Update comment to reflect code logic in Ownable.sol #4369
Conversation
The initial owner is not automatically assigned to the deployer's address. Rather, the initial owner is set to the constructor argument provided by the deployer. (The code was modified in commit 13d5e04 but the comment wasn't.)
|
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.
Hey @s-tikhomirov, Github doesn't allow suggestions 3 lines further than the change, but I think we should also update the NatSpec on top of the contract itself where it reads:
By default, the owner account will be the one that deploys the contract.
We've already received comments about this inaccuracy, so just flagging it before merging.
Fix NatSpec comment regarding the initial owner
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.
Thanks @s-tikhomirov!
Congrats, your important contribution to this open-source project has earned you a GitPOAP! GitPOAP: 2023 OpenZeppelin Contracts Contributor: Head to gitpoap.io & connect your GitHub account to mint! Learn more about GitPOAPs here. |
The initial owner is not automatically assigned to the deployer's address. Rather, the initial owner is set to the constructor argument provided by the deployer.
(The code was modified in commit 13d5e04 but the comment wasn't.)
Fixes #????
PR Checklist
npx changeset add
)