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

chore: update l1/2 name for dataEntities #340

Merged
merged 13 commits into from
Nov 2, 2023
Merged

chore: update l1/2 name for dataEntities #340

merged 13 commits into from
Nov 2, 2023

Conversation

douglance
Copy link
Contributor

@douglance douglance commented Sep 25, 2023

closes #338

Base automatically changed from dl/#336 to v4 October 19, 2023 14:19
@douglance douglance marked this pull request as ready for review October 19, 2023 14:29
L2Network,
getL1Network,
getL2Network,
ParentChains as L1Networks,
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

@douglance douglance Oct 23, 2023

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,
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes ser

@@ -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[]
Copy link
Contributor

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?

Copy link
Contributor Author

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

@douglance douglance merged commit 09a6eb0 into v4 Nov 2, 2023
7 checks passed
douglance added a commit that referenced this pull request Nov 15, 2023
* 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
douglance added a commit that referenced this pull request Nov 15, 2023
* 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
@douglance douglance deleted the dl/338 branch April 22, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants