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.

Sign up for GitHub

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

Jump to bottom

feat: incoming transactions by network #7154

Merged
merged 19 commits into from
Oct 19, 2023
Merged

feat: incoming transactions by network #7154

merged 19 commits into from
Oct 19, 2023

Conversation

tommasini
Copy link
Contributor

@tommasini tommasini commented Sep 6, 2023 •
edited
Loading

Description
More info here

Core pr: MetaMask/core#1659
Screenshots/Recordings

If applicable, add screenshots and/or recordings to visualize the before and after of your change

Issue

fixes #???

Checklist

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

@tommasini tommasini requested a review from a team as a code owner September 6, 2023 17:11
@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

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.

@gauthierpetetin gauthierpetetin added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Sep 6, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 6, 2023

Please retry analysis of this Pull-Request directly on SonarCloud.

@codecov-commenter
Copy link

codecov-commenter commented Sep 6, 2023 •
edited
Loading

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (bab39f1) 34.59% compared to head (99bd33c) 34.62%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7154      +/-   ##
==========================================
+ Coverage   34.59%   34.62%   +0.02%     
==========================================
  Files        1019     1019              
  Lines       27192    27234      +42     
  Branches     2212     2218       +6     
==========================================
+ Hits         9408     9430      +22     
- Misses      17296    17315      +19     
- Partials      488      489       +1
Files Coverage Δ
app/actions/privacy/index.js 0.00% <ø> (ø)
...Settings/NetworksSettings/NetworkSettings/index.js 2.64% <ø> (ø)
...omponents/Views/Settings/NetworksSettings/index.js 3.80% <ø> (ø)
...ttings/SecuritySettings/SecuritySettings.styles.ts 100.00% <ø> (ø)
app/core/Engine.ts 53.94% <100.00%> (+0.20%) ⬆️
app/reducers/privacy/index.js 50.00% <ø> (+4.54%) ⬆️
app/selectors/preferencesController.ts 100.00% <100.00%> (ø)
app/components/Nav/Main/index.js 7.31% <0.00%> (-0.06%) ⬇️
app/store/migrations.js 41.92% <72.72%> (+1.36%) ⬆️
...ews/Settings/SecuritySettings/SecuritySettings.tsx 51.55% <45.71%> (-2.02%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

NicolasMassart
NicolasMassart reviewed Sep 12, 2023
View reviewed changes
Copy link
Contributor

@NicolasMassart NicolasMassart left a comment

Choose a reason for hiding this comment

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

Just a few suggestions to make the implem. consistent with the existing code and guidelines and a small typo to fix in the patch.
I haven't checked the result in the app so QA needed here.

app/store/migrations.test.js Outdated Show resolved Hide resolved
app/core/Engine.ts Outdated Show resolved Hide resolved
app/components/Nav/Main/index.js Outdated Show resolved Hide resolved
patches/@metamask+preferences-controller+3.0.0.patch Show resolved Hide resolved
patches/@metamask+preferences-controller+3.0.0.patch Outdated Show resolved Hide resolved
patches/@metamask+preferences-controller+3.0.0.patch Outdated Show resolved Hide resolved
patches/@metamask+preferences-controller+3.0.0.patch Outdated Show resolved Hide resolved
NicolasMassart
NicolasMassart previously approved these changes Sep 12, 2023
View reviewed changes
Copy link
Contributor

@NicolasMassart NicolasMassart left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@gauthierpetetin gauthierpetetin added needs-qa Any New Features that needs a full manual QA prior to being added to a release. in-progress and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. in-progress labels Sep 12, 2023
NicolasMassart
NicolasMassart previously approved these changes Sep 20, 2023
View reviewed changes
Copy link
Contributor

@NicolasMassart NicolasMassart left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@gauthierpetetin gauthierpetetin removed the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Sep 25, 2023
@gauthierpetetin gauthierpetetin added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed issues-found labels Sep 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

44.6% 44.6% Coverage
0.0% 0.0% Duplication

@cortisiko cortisiko added QA Passed A successful QA run through has been done and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Oct 19, 2023
cortisiko
cortisiko approved these changes Oct 19, 2023
View reviewed changes
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

This PR is ✅

@tommasini tommasini merged commit d2e5308 into main Oct 19, 2023
26 of 27 checks passed
@tommasini tommasini deleted the feat/1188-incoming-transactions branch October 19, 2023 14:48
@tommasini tommasini added the release-7.10.0 Issue or pull request that will be included in release 7.10.0 label Oct 19, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2023
@cortisiko cortisiko restored the feat/1188-incoming-transactions branch October 19, 2023 18:51
@cortisiko cortisiko deleted the feat/1188-incoming-transactions branch October 19, 2023 18:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sethkfman sethkfman sethkfman left review comments

@cortisiko cortisiko cortisiko approved these changes

@NicolasMassart NicolasMassart NicolasMassart left review comments

Assignees
No one assigned
Labels
QA Passed A successful QA run through has been done release-7.10.0 Issue or pull request that will be included in release 7.10.0 team-mobile-platform
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tommasini @codecov-commenter @NicolasMassart @sethkfman @cortisiko @gauthierpetetin