-
Notifications
You must be signed in to change notification settings - Fork 177
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
chore: update l1/2 name for dataEntities #340
Conversation
L2Network, | ||
getL1Network, | ||
getL2Network, | ||
ParentChains as L1Networks, |
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.
Should we also export these parent/child versions? Will we get rid of the old L1Network variant and bump a major?
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.
All the import/export aliases that are used during this renaming process will eventually be removed.
We're using them to reduce the number of changes in each PR.
These changes are targeting a major bump release.
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.
There will also be a big file renaming PR at the end.
L2Network, | ||
getL1Network, | ||
getL2Network, | ||
ParentChain as L1Network, |
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.
Will the usages of the neworks in the imported files be renamed in a separate pr?
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.
yes ser
src/lib/dataEntities/networks.ts
Outdated
@@ -21,13 +21,13 @@ import { ArbSdkError } from '../dataEntities/errors' | |||
import { SEVEN_DAYS_IN_SECONDS } from './constants' | |||
import { RollupAdminLogic__factory } from '../abi/factories/RollupAdminLogic__factory' | |||
|
|||
export interface L1Network extends Network { | |||
export interface ParentChain extends Network { | |||
partnerChainIDs: number[] |
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.
Should we rename this to childChainIds, parentChainId on the child chain?
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 really wanted to, though I didn't want to make too many changes, but since you suggested it... updated in 908fa85
* chore: renames L1/2 to parentChain/chain in inbox * update to use new childChain name with ArbitrumChain type * updates name of methods on inbox to use childChain * Update childChainTransactionRequest * remove `network` * update names with from pr feedback * chore: update l1/2 name for dataEntities * removes network * fix child renaming * fix getChildChain * fix integration test * renames partner to parent/child
* chore: renames L1/2 to parentChain/chain in inbox * update to use new childChain name with ArbitrumChain type * updates name of methods on inbox to use childChain * Update childChainTransactionRequest * remove `network` * update names with from pr feedback * chore: update l1/2 name for dataEntities * removes network * fix child renaming * fix getChildChain * fix integration test * renames partner to parent/child
closes #338