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: Remove setRelease and setDist, have auto release passed to native #213

Merged
merged 5 commits into from
Mar 22, 2021

Conversation

jennmueng
Copy link
Member

@jennmueng jennmueng commented Mar 19, 2021

In previous versions, we hadsetRelease and setDist which set __sentry_release and __sentry_dist as an extra, we remove these methods in favor of passing release and dist to init. This removes the existing code on the iOS native bridge that handles this, and we did not add support for this on the Android bridge.

We also have the SENTRY_RELEASE environment variable that is built by the script at build time and matches the sourcemaps set the release on init instead of setting it in a JS event processor. This way we have the auto release set on native events as well.

Closes #196 (Instead of deprecating, we should remove entirely given that this is a major bump anyways)

Testing

Added tests to test that SENTRY_RELEASE is passed as release when initing the SDK, and that a user-set release takes precedent over the default auto release.

Tested on sample app on iOS and Android.

As the Android native bridge has no support for setRelease and setDist which set __sentry_release and __sentry_dist as an extra, we remove these methods in favor of passing release and dist to init.
We also have the SENTRY_RELEASE environment variable that is set at buildtime set the release on init instead.

Closes #196
@jennmueng jennmueng self-assigned this Mar 19, 2021
@jennmueng jennmueng added this to Waiting for Review in Mobile Platform Team Archived Mar 19, 2021
@jennmueng jennmueng requested a review from a team March 19, 2021 11:09
Copy link

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

@jennmueng jennmueng merged commit 09da745 into main Mar 22, 2021
Mobile Platform Team Archived automation moved this from Waiting for Review to Done Mar 22, 2021
@bruno-garcia bruno-garcia deleted the jenn/set-release branch March 23, 2021 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Deprecate setRelease, setDist and migrate users to passing release/dist in init. (Works already)
2 participants