Skip to content
This repository was archived by the owner on Oct 22, 2024. It is now read-only.

Fix version on Segment for desktop app#430

Merged
bergarces merged 2 commits intomainfrom
feat/segment-desktop-enabled-trait
Jan 19, 2023
Merged

Fix version on Segment for desktop app#430
bergarces merged 2 commits intomainfrom
feat/segment-desktop-enabled-trait

Conversation

@bergarces
Copy link
Copy Markdown
Contributor

@bergarces bergarces commented Jan 17, 2023

Context

Fix how version is displayed for Segment metrics that come from MetametricsController when desktop app is paired.

Changes

App

  • Add version_name property so that the desktop version is displayed.
  • Add METAMASK_ENVIRONMENT env to our package pipelines so that the version is built as expected within the MetametricsController.

Links to the relevant code that affects these changes provided in comments.

Screenshots

image

Copy link
Copy Markdown
Contributor Author

@bergarces bergarces Jan 17, 2023

Choose a reason for hiding this comment

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

This is needed so that browser.runtime.getManifest() returns a version_name field that, for extension builds, is based on the version from package.json + build type (e.g. 10.23.2-desktop.0, 10.23.2-flask.0).

With this change, for the desktop app right now, version_name: 0.0.1-desktop.0.

The version is passed to the MetaMetrics controller and used to report segment stats (https://github.com/MetaMask/desktop-extension/blob/main/app/scripts/metamask-controller.js#L424)

@bergarces bergarces force-pushed the feat/segment-desktop-enabled-trait branch from 0eb9144 to f325cc4 Compare January 17, 2023 13:59
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We also need to ensure METAMASK_ENVIRONMENT is set as 'production' for the production build, as otherwise it appends an unknown to the version (https://github.com/MetaMask/desktop-extension/blob/main/app/scripts/controllers/metametrics.js#L125).

We can also set METAMASK_ENVIRONMENT in our local build if we want to avoid -unknown being appended.

@bergarces bergarces changed the title chore: update submodule Fix version on Segment for desktop app Jan 17, 2023
@bergarces bergarces force-pushed the feat/segment-desktop-enabled-trait branch from f325cc4 to b2506ed Compare January 19, 2023 10:33
@bergarces bergarces marked this pull request as ready for review January 19, 2023 10:33
@bergarces bergarces requested a review from a team January 19, 2023 10:33
@bergarces bergarces force-pushed the feat/segment-desktop-enabled-trait branch from b2506ed to e397ca3 Compare January 19, 2023 10:35
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I did this a while back but I can't recall if this is the browser version or if it should be the numerical portion of the version_name below?

Copy link
Copy Markdown
Contributor Author

@bergarces bergarces Jan 19, 2023

Choose a reason for hiding this comment

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

It is the numerical portion of the version_name indeed.

version: "10.23.2.0"
version_name: "10.23.2-desktop.0"

Probably best to match the behaviour whilst we are at it just in case.

@bergarces bergarces force-pushed the feat/segment-desktop-enabled-trait branch from e397ca3 to 5e38559 Compare January 19, 2023 11:45
@bergarces bergarces merged commit 3eabd53 into main Jan 19, 2023
@bergarces bergarces deleted the feat/segment-desktop-enabled-trait branch January 19, 2023 13:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants