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

refactor: Consistent provider config naming #6880

Merged
merged 1 commit into from
Jul 27, 2023

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 24, 2023

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

The providerConfig property of the NetworkController used to be named provider, which was very easily confused with the provider instance (and with various other things that we call "provider"). When that renaming happened, not all local variables and function names were updated accordingly.

All references to providerConfig now use the term "providerConfig".

Issue

This change was done to simplify PR #6872, which is part of https://github.com/MetaMask/mobile-planning/issues/798

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@Gudahtt Gudahtt force-pushed the consistent-provider-config-naming branch from f28432a to da122aa Compare July 24, 2023 22:54
@Gudahtt Gudahtt added No QA Needed Apply this label when your PR does not need any QA effort. needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Jul 24, 2023
@Gudahtt Gudahtt marked this pull request as ready for review July 24, 2023 22:54
@Gudahtt Gudahtt requested a review from a team as a code owner July 24, 2023 22:54
@Gudahtt
Copy link
Member Author

Gudahtt commented Jul 24, 2023

@Gudahtt Gudahtt force-pushed the consistent-provider-config-naming branch 4 times, most recently from be52b17 to ecbfebd Compare July 25, 2023 16:32
@Gudahtt Gudahtt force-pushed the consistent-provider-config-naming branch 8 times, most recently from a020b22 to 80def06 Compare July 27, 2023 11:52
The `providerConfig` property of the `NetworkController` used to be
named `provider`, which was very easily confused with the `provider`
instance (and with various other things that we call "provider"). When
that renaming happened, not all local variables and function names were
updated accordingly.

All references to `providerConfig` now use the term "providerConfig".

This change was done to simplify PR #6872, which is part of
MetaMask/mobile-planning#798
@Gudahtt Gudahtt force-pushed the consistent-provider-config-naming branch from 80def06 to 01c162a Compare July 27, 2023 15:59
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@Cal-L Cal-L removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jul 27, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

39.3% 39.3% Coverage
0.0% 0.0% Duplication

@Gudahtt Gudahtt merged commit c2e6734 into main Jul 27, 2023
@Gudahtt Gudahtt deleted the consistent-provider-config-naming branch July 27, 2023 17:16
@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2023
@metamaskbot metamaskbot added the release-7.5.0 Issue or pull request that will be included in release 7.5.0 label Jul 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
No QA Needed Apply this label when your PR does not need any QA effort. release-7.5.0 Issue or pull request that will be included in release 7.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants