Skip to content

add rollbackToPreviousProvider method - #1132

Merged
adonesky1 merged 3 commits into
mainfrom
add-rollback-to-previous-provider
Mar 22, 2023
Merged

add rollbackToPreviousProvider method#1132
adonesky1 merged 3 commits into
mainfrom
add-rollback-to-previous-provider

Conversation

@adonesky1

@adonesky1adonesky1 commented Mar 15, 2023

Copy link
Copy Markdown
Contributor

  • ADDED:

Resolves#1024

@adonesky1
adonesky1force-pushed the add-rollback-to-previous-provider branch 2 times, most recently from 3492ae6 to ae2603dCompareMarch 16, 2023 16:22
@adonesky1
adonesky1 marked this pull request as ready for review March 16, 2023 16:23
@adonesky1
adonesky1 requested review from a team, BelfordZ, Gudahtt and mcmireMarch 16, 2023 16:23
mcmire
mcmire reviewed Mar 16, 2023
View reviewed changes
Comment threadpackages/network-controller/src/NetworkController.ts Outdated
Comment threadpackages/network-controller/tests/NetworkController.test.ts Outdated
Comment threadpackages/network-controller/tests/NetworkController.test.ts Outdated
Comment threadpackages/network-controller/src/NetworkController.ts Outdated
Comment threadpackages/network-controller/src/NetworkController.ts Outdated
Comment threadpackages/network-controller/src/NetworkController.ts Outdated
@adonesky1
adonesky1force-pushed the migrate-network-configs-to-network-controller branch 3 times, most recently from f659097 to 479e349CompareMarch 16, 2023 22:59
@adonesky1
adonesky1force-pushed the add-rollback-to-previous-provider branch 5 times, most recently from cc23734 to b0f9743CompareMarch 17, 2023 22:54
mcmire
mcmire reviewed Mar 20, 2023
View reviewed changes

@mcmiremcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is looking good! Just a few comments

Comment threadpackages/assets-controllers/src/NftController.ts Outdated

@mcmiremcmireMar 20, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice! Yeah this is much better.

Comment threadpackages/controller-utils/src/types.ts Outdated

@mcmiremcmireMar 20, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thoughts on making this a function instead of a const? I believe this will enforce that this is JSDoc'd as well.

@adonesky1adonesky1Mar 21, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

done here: 19c536a

Comment threadpackages/controller-utils/jest.config.js Outdated
@adonesky1
adonesky1 marked this pull request as draft March 20, 2023 18:04
@adonesky1
adonesky1force-pushed the add-rollback-to-previous-provider branch from ceb710d to 49b5222CompareMarch 21, 2023 14:35
@adonesky1
adonesky1 marked this pull request as ready for review March 21, 2023 18:23
@adonesky1
adonesky1 requested a review from mcmireMarch 21, 2023 18:23
mcmire
mcmire previously approved these changes Mar 21, 2023
View reviewed changes

@mcmiremcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks great!

@adonesky1

adonesky1 commented Mar 21, 2023

Copy link
Copy Markdown
ContributorAuthor

@mcmire thank you for the reviews! Since this is PR based on #1064 I'm going to wait til that merges to merge this!

@mcmiremcmire linked an issue Mar 21, 2023 that may be closed by this pull request
NetworkController API normalization: Add rollbackToPreviousProvider method. #1024
Closed
Base automatically changed from migrate-network-configs-to-network-controller to mainMarch 22, 2023 19:38
@adonesky1
adonesky1force-pushed the add-rollback-to-previous-provider branch from 1500a6e to 8061393CompareMarch 22, 2023 21:08
mcmire
mcmire reviewed Mar 22, 2023
View reviewed changes

@mcmiremcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One small thing, but other than that, good to go.

Comment threadpackages/network-controller/tests/provider-api-tests/shared-tests.ts Outdated
@@ -1,5 +1,6 @@
/* eslint-disable jest/require-top-level-describe, jest/no-export, jest/no-identical-title, jest/no-if */

import { NetworkType } from '@metamask/controller-utils/src';

@mcmiremcmireMar 22, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are we exporting this type from controller-utils? If so, I think we can drop the /src.

@adonesky1adonesky1Mar 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

oh... weird not sure how that happened!

@adonesky1adonesky1Mar 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

fixed here: da27c40

mcmire
mcmire approved these changes Mar 22, 2023
View reviewed changes

@mcmiremcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good!

@adonesky1
adonesky1 merged commit 99df8b5 into mainMar 22, 2023
@adonesky1
adonesky1 deleted the add-rollback-to-previous-provider branch March 22, 2023 21:28
@legobeatlegobeat mentioned this pull request Apr 25, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcmiremcmiremcmire approved these changes
@BelfordZBelfordZAwaiting requested review from BelfordZ
@GudahttGudahttAwaiting requested review from Gudahtt

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

NetworkController API normalization: Add rollbackToPreviousProvider method.

2 participants

@adonesky1@mcmire