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

Master sync following v10.28.2 and v10.28.3 #18656

Merged
merged 12 commits into from
Apr 19, 2023
Merged

Master sync following v10.28.2 and v10.28.3 #18656

merged 12 commits into from
Apr 19, 2023

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Apr 19, 2023

Explanation

Screenshots/Screencaps

Before

After

Manual Testing Steps

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.

metamaskbot and others added 11 commits April 6, 2023 15:14
@metamaskbot
Version v10.28.2
0cb53f7
@danjm @legobeat
Fix switch-ethereum-chain handler by passing configuration id to setA… (
43d5bf1 
#18483)

* Fix switch-ethereum-chain handler by passing configuration id to setActiveNetwork

* fix e2e test

* Fix e2e tests

* Update test/e2e/tests/switch-custom-network.spec.js

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>

* Revert "Update test/e2e/tests/switch-custom-network.spec.js"

This reverts commit be533ff.

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
@vthomas13 @legobeat @danjm
Bumping notification id's to 18 & 19 (#18460)
227af24 
Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Co-authored-by: Dan J Miller <danjm.com@gmail.com>
@danjm
Fix locale file issues on chrome store (#18487)
d8e7918
@danjm
Update change log
afc7ec3
@danjm
Merge pull request #18486 from MetaMask/Version-v10.28.2
22cb8ce 
Version v10.28.2 RC
@metamaskbot
Version v10.28.3
2cc41c9
@danjm @legobeat @Gudahtt
Ensure that all networkConfiguration object in networkController stat…
6a14f12 
…e have an id (#18513)

* Ensure that all networkConfiguration object in networkController state have an id

* Lint fix

* Update app/scripts/migrations/084.ts

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

* Add unit tests for error cases

* Simplify code

* Remove unnecessary any typing

* Fix network controller type checking

* Lint fix

* Improve typing

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
@danjm
Fix changelog for v10.28.3
8220051
@legobeat @danjm
deps/security: vm2@3.9.11->3.9.15 (#18512)
0ef2680 
Fixes CVE-2023-29017 / GHSA-7jxr-cg7f-gpgv
@danjm
Merge pull request #18535 from MetaMask/Version-v10.28.3
2fb016e 
Version v10.28.3 RC
@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.

@danjm danjm changed the base branch from develop to switch-migration-83-84 April 19, 2023 09:21
@danjm danjm changed the base branch from switch-migration-83-84 to develop April 19, 2023 09:21
@danjm
Copy link
Contributor Author

danjm commented Apr 19, 2023

Let's merge #18655 first, to simplify the diff on this PR

@legobeat
Copy link
Contributor

@danjm Perhaps a branch protection rule and/or something like MetaMask/eth-phishing-detect#12210 would help moving forward?

@danjm danjm marked this pull request as ready for review April 19, 2023 16:14
@danjm danjm requested a review from a team as a code owner April 19, 2023 16:14
@danjm
Copy link
Contributor Author

danjm commented Apr 19, 2023

@danjm Perhaps a branch protection rule and/or something like MetaMask/eth-phishing-detect#12210 would help moving forward?

@legobeat maybe... what problem would we be trying to solve exactly? Or what improvement would we be aiming for?

@danjm danjm changed the title Master sync Master sync following v10.28.2 and v10.28.3 Apr 19, 2023
Gudahtt
Gudahtt approved these changes Apr 19, 2023
View reviewed changes
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@metamaskbot
Copy link
Collaborator

Builds ready [53faf79]
Page Load Metrics (1556 ± 38 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint95152114168
domContentLoaded1394166215457536
load1395167815567938
domInteractive1394166115457536
Bundle size diffs
  • background: 0 bytes
  • ui: 0 bytes
  • common: 0 bytes

@danjm danjm merged commit 5bb8797 into develop Apr 19, 2023
@danjm danjm deleted the master-sync branch April 19, 2023 19:19
@github-actions github-actions bot locked and limited conversation to collaborators Apr 19, 2023
@legobeat
Copy link
Contributor

legobeat commented Apr 19, 2023 •
edited
Loading

@danjm I assume that:

  • master is actually not supposed to have active branches merged to it and if it happens it's typically a mistake
    • A BP can prevent accidental merges to master by preventing PRs merging to the wrong base
  • This PR is just cleaning up mistakes people made by confusing the branches
    • If not, what's the actual motivation behind this PR?
  • Keeping master tracking develop minimizes drift and reduces any time wasted on syncing over changes
    • That's what the linked PR does

Makes sense?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Gudahtt Gudahtt Gudahtt approved these changes

@PeterYinusa PeterYinusa PeterYinusa approved these changes

@NiranjanaBinoy NiranjanaBinoy Awaiting requested review from NiranjanaBinoy NiranjanaBinoy is a code owner automatically assigned from MetaMask/extension-devs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@danjm @legobeat @metamaskbot @Gudahtt @PeterYinusa @vthomas13