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

fix(3742): change getMetaMetricsId to only sync func type #5108

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

DDDDDanica
Copy link
Contributor

@DDDDDanica DDDDDanica commented Jan 7, 2025

Explanation

Changes introduced in this PR including:

  • remove fallback of metaMetricsId and rely on client side always returning a value
  • refactor getMetaMetricsId to only handle synchronous (extension) function

References

Addressed feedback #5051 (comment)

@metamask/remote-feature-flag-controller

  • CHANGED: Modify signature of getMetaMetricsId to handle only synchronous function
  • CHANGED: Remove fallback of metaMetricsId

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking 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!

Nit for the changelog in the PR description; both seem appropriate to categorize as changes, the changed getMetaMetricsId signature doesn't seem like a new feature to me.

@DDDDDanica
Copy link
Contributor Author

@Gudahtt thanks for suggestions, modified !

@DDDDDanica DDDDDanica merged commit 7c38c24 into main Jan 8, 2025
121 checks passed
@DDDDDanica DDDDDanica deleted the fix/3742-getMetaMetricsId branch January 8, 2025 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants