Skip to content
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

Rename DelegationTokenStorage #53

Merged
merged 3 commits into from
Oct 23, 2024
Merged

Rename DelegationTokenStorage #53

merged 3 commits into from
Oct 23, 2024

Conversation

colin-morpho
Copy link
Contributor

For consistency, we rename the DelegationToken storage.

@colin-morpho colin-morpho marked this pull request as ready for review October 23, 2024 08:07
@colin-morpho colin-morpho changed the title Rename DelegationToken storage name Rename DelegationTokenStorage Oct 23, 2024
MathisGD
MathisGD previously approved these changes Oct 23, 2024
MerlinEgalite
MerlinEgalite previously approved these changes Oct 23, 2024
QGarchery
QGarchery previously approved these changes Oct 23, 2024
src/DelegationToken.sol Show resolved Hide resolved
0xd583ef41af40c9ecf9cd08176e1b50741710eaecf057b22e93a6b99fa47a6400;

/* STORAGE LAYOUT */

/// @custom:storage-location erc7201:DelegationToken
struct ERC20DelegatesStorage {
struct DelegationTokenStorage {
Copy link
Contributor

@Jean-Grimal Jean-Grimal Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this storage is only about delegation

Suggested change
struct DelegationTokenStorage {
struct DelegationStorage {

Or we can just do like OZ and stay with the exact contract name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It matches the name of the contract though.

@MerlinEgalite MerlinEgalite merged commit 1bf203b into main Oct 23, 2024
2 checks passed
@MerlinEgalite MerlinEgalite deleted the fix/rename-storage branch October 23, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants