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

Remove unused methods in the transaction controller #20194

Merged
merged 3 commits into from
Jul 28, 2023

Conversation

vinistevam
Copy link
Contributor

@vinistevam vinistevam commented Jul 26, 2023

Explanation

In line with the ongoing initiative to align the transaction controller between the extension and the core repository, this pull request addresses code maintainability by removing two unused methods.

Removed Methods:

  • updateTransactionUserSettings
  • updateTransactionEstimatedBaseFee

These methods are no longer being used, and their removal will help streamline the codebase and ensure better maintainability going forward. The changes have been carefully tested to avoid any impact on existing functionalities.

Manual Testing Steps

  • Go to Test dApp
  • Perform all existing transaction support

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Collaborator

Builds ready [8aacca4]
Page Load Metrics (1709 ± 129 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1103551555627
domContentLoaded142723421707268129
load142823431709268129
domInteractive142723421707268129
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -340 Bytes (-0.01%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Merging #20194 (ec2306a) into develop (d262acb) will decrease coverage by 0.01%.
Report is 1 commits behind head on develop.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop   #20194      +/-   ##
===========================================
- Coverage    68.70%   68.69%   -0.01%     
===========================================
  Files          986      986              
  Lines        37817    37803      -14     
  Branches     10116    10116              
===========================================
- Hits         25979    25965      -14     
  Misses       11838    11838              
Files Changed Coverage Δ
app/scripts/controllers/transactions/index.js 72.34% <ø> (-0.45%) ⬇️

@vinistevam vinistevam changed the title Remove unused methods Remove unused methods in the transaction controller Jul 26, 2023
@metamaskbot
Copy link
Collaborator

Builds ready [ec2306a]
Page Load Metrics (1637 ± 71 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint107177132157
domContentLoaded14132017163714871
load14132017163714871
domInteractive14132017163714871
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -644 Bytes (-0.01%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@vinistevam vinistevam marked this pull request as ready for review July 26, 2023 09:32
@vinistevam vinistevam requested a review from a team as a code owner July 26, 2023 09:32
@vinistevam vinistevam merged commit 553da10 into develop Jul 28, 2023
@vinistevam vinistevam deleted the 957-remove-unused-methods-tx-controller branch July 28, 2023 09:01
@github-actions github-actions bot locked and limited conversation to collaborators Jul 28, 2023
@metamaskbot metamaskbot added the release-10.36.0 Issue or pull request that will be included in release 10.36.0 label Jul 28, 2023
@Gudahtt Gudahtt added release-11.1.0 Issue or pull request that will be included in release 11.1.0 and removed release-10.36.0 Issue or pull request that will be included in release 10.36.0 labels Sep 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-11.1.0 Issue or pull request that will be included in release 11.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants