-
Notifications
You must be signed in to change notification settings - Fork 24
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 To ICM #652
Rename To ICM #652
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.
Honestly... the more time I stare at all these changes, the less likely I am to catch something that's off. I think we should just merge this TBH. If there are mistakes or missed changes (which wouldn't show up in this PR anyway), we can change them as needed.
@@ -118,7 +118,7 @@ abstract contract ValidatorManager is Initializable, ContextUpgradeable, IValida | |||
onlyInitializing | |||
{ | |||
ValidatorManagerStorage storage $ = _getValidatorManagerStorage(); | |||
$._subnetID = settings.subnetID; | |||
$._l1ID = settings.l1ID; |
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.
😂 this hurts my eyes
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.
I don't disagree...
🤔 all these files still have the word
|
I don't know if this is actually helpful, but all these files have the word "subnet" in them
(though I know that some of them are because of the |
Are we renaming the module in |
847dad2
to
3ef0507
Compare
Why this should be merged
Closes #620
It's probably easier to view this PR commit by commit.
There are a few quirks of the renaming. Now that
Warp
andTeleporter
are bothICM
, it's hard to documentTeleporter
. In most spots, I opted for specifyingTeleporterMessenger
the contract, to disambiguate it fromICM at large
.There are some places where our upstream dependencies haven't renamed
subnet
fields tol1
, are some of them going to stay that way? Is warp going to get renamed in the dependencies?