-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Version v12.13.1 RC #30812
Merged
Merged
Version v12.13.1 RC #30812
+1,403
−578
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Truly solving MetaMask/MetaMask-planning#3932 required us to clear `previousUserTraits` from metametrics controller state. That property just functions as a cache, and we could actually maintain it in memory. The reason we need to clear it is that there are many users who had the `has_marketing_consent` trait populated to `true` when the application had a bug that prevented that trait from being submitted to segment. That bug is fixed, so new users don't hit that problem. However, to correctly track existing users that have `has_marketing_consent` set to true, we need the comparison of previous and current user traits in the `_buildUserTraitsObject` function to fail its equality check. To do that, we are clearing `previousUserTraits` so that, upon update of the extension and when the first metrics event is set to segment, the current user trait values will compare to undefined, and they will all be submitted to segment, including `has_marketing_consent`. [](https://codespaces.new/MetaMask/metamask-extension/pull/30621?quickstart=1) Fixes: 1. Install a build from this branch 2. Open the background console and the network tab 3. Start onboarding. On the metametrics screen, click the checkbox and then "I agree" 4. The network tab should now include a request to segment with user traits, where has_marketing_consent is set to true -- 1. Follow the above steps on the v12.9.3 build (step 4 will fail) 2. Update the version of that install to the build from this branch 3. Log in. 4. The network tab should now include a request to segment with user traits, where has_marketing_consent is set to true <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> <!-- [screenshots/recordings] --> <!-- [screenshots/recordings] --> - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
- **prep-deps** - There's a new workflow that parallels the CircleCI workflow. `prep-deps` runs first and caches the result, then `prep-build-test-webpack` makes an artifact, then the benchmarks run. - Several of the existing workflows were changed to now depend on `prep-deps` - Many of the file changes are just using the new `checkout-and-setup@caching` and deleting the independent "Checkout repository" step - ~~**vars.USE_CACHING**~~ No longer using this - ~~This allows us to toggle the caching feature on and off in a centralized way. I left instructions as a comment in `main.yml` for how to toggle it. We can discuss implementing this in a different way, but passing this variable through different workflows is actually kind of a pain (that's the first way I implemented it). Several of the jobs are 30-60 seconds faster when this is turned on.~~ - **~~test-short-suite~~ repository-health-checks** - I think this could be slightly controversial, but I think it's for the better. - I noticed that there were 7 individual really short tests, that each ran on their own VM. Each VM instance took about 1m30s to execute. I combined these together into a single VM that runs all the short tests in sequence, and takes about 1m20s combined. The `if: always()` statements make all independent tests run, even if one fails. - The 7 jobs were: `test-lint-shellcheck, test-lint-changelog, test-lint-lockfile, test-deps-audit, test-yarn-dedupe, test-deps-depcheck, validate-lavamoat-allow-scripts` - If this were running on VMs that cost money, this would be a no-brainer. As is on GitHub runners, it doesn't really cost us any time or money. All we can really say is 7 separate jobs burn fossil fuels for a slight bit of convenience in terms of seeing individual test failures in the GitHub PR page. - ~~Depends on #29955~~ **MERGED** - ~~Merge this `caching` branch of `github-tools`: MetaMask/github-tools#42~~ **MERGED** - ~~Change `checkout-and-setup@caching` and `checkout-and-setup-secure@caching` to new `@hash`~~ [](https://codespaces.new/MetaMask/metamask-extension/pull/29979?quickstart=1) Split off from: #29955 <!-- --> --------- Co-authored-by: Hassan Malik <[email protected]>
…est (#30680) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This temporarily disables the "Test Snap update via snaps component" test because it's broken on `main`. [](https://codespaces.new/MetaMask/metamask-extension/pull/30680?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** We temporarily ignore the `'@trezor/connect-web` audit failure to unblock ci, as upgrading to the new version breaks the webpack build. ``` └─ @trezor/connect-web ├─ ID: @trezor/connect-web (deprecation) ├─ Issue: This version is no longer supported ├─ Severity: moderate ├─ Vulnerable Versions: 9.4.7 │ ├─ Tree Versions │ └─ 9.4.7 │ └─ Dependents └─ metamask-crx@workspace:. ``` [This issue](#30851) is created in order to upgrade to the latest version and remove the entry from the ignore list. [](https://codespaces.new/MetaMask/metamask-extension/pull/30850?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Check yarn audit gh action ## **Screenshots/Recordings** ### Before  ### After  ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Frederik Bolding <[email protected]>
chore(cherr-pick): a new prep-deps workflow with caching (#29979)
Builds ready [119e70c]
Page Load Metrics (1732 ± 72 ms)
|
danjm
approved these changes
Mar 11, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📦 🚀