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

Don't override sdk version with timber #2450

Merged
merged 4 commits into from
Jan 3, 2023
Merged

Don't override sdk version with timber #2450

merged 4 commits into from
Jan 3, 2023

Conversation

romtsn
Copy link
Member

@romtsn romtsn commented Jan 3, 2023

📜 Description

We should not override the sdk version/name, because Timber is just an integration as opposed to other "sdk-like" things, like spring-boot or log4j. We can still query the packages and see who's using timber (and soon, integrations).

This way we avoid the weird sentry.java.android.timber name in the SDK description (should be sentry.java.android)
image

💡 Motivation and Context

Closes #1893

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

@github-actions
Copy link
Contributor

github-actions bot commented Jan 3, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 361.02 ms 399.46 ms 38.44 ms
Size 1.73 MiB 2.33 MiB 616.87 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
4a9c176 319.77 ms 363.20 ms 43.43 ms
f1150bc 352.62 ms 453.27 ms 100.65 ms
f809aac 301.51 ms 346.60 ms 45.09 ms
4a9c176 336.33 ms 384.73 ms 48.41 ms
5fd37fa 334.02 ms 401.10 ms 67.08 ms
703d523 358.00 ms 369.94 ms 11.94 ms
81a1a6c 328.73 ms 421.28 ms 92.55 ms
a04f788 321.78 ms 354.12 ms 32.35 ms
95647bd 349.76 ms 397.92 ms 48.16 ms
ecf9680 303.12 ms 346.35 ms 43.23 ms

App size

Revision Plain With Sentry Diff
4a9c176 1.73 MiB 2.33 MiB 612.69 KiB
f1150bc 1.73 MiB 2.33 MiB 615.66 KiB
f809aac 1.73 MiB 2.32 MiB 608.63 KiB
4a9c176 1.73 MiB 2.33 MiB 612.69 KiB
5fd37fa 1.73 MiB 2.33 MiB 616.48 KiB
703d523 1.73 MiB 2.33 MiB 613.23 KiB
81a1a6c 1.73 MiB 2.32 MiB 612.47 KiB
a04f788 1.73 MiB 2.32 MiB 609.88 KiB
95647bd 1.73 MiB 2.33 MiB 616.54 KiB
ecf9680 1.73 MiB 2.32 MiB 612.39 KiB

Previous results on branch: fix/timber-sdk-version

Startup times

Revision Plain With Sentry Diff
222c9ff 324.50 ms 346.47 ms 21.97 ms
a9e5c7c 309.66 ms 367.50 ms 57.84 ms

App size

Revision Plain With Sentry Diff
222c9ff 1.73 MiB 2.33 MiB 616.54 KiB
a9e5c7c 1.73 MiB 2.33 MiB 616.54 KiB

Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

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

Neat! Less code, less bugs 😄

@romtsn romtsn enabled auto-merge (squash) January 3, 2023 20:47
@romtsn romtsn merged commit 8ade225 into main Jan 3, 2023
@romtsn romtsn deleted the fix/timber-sdk-version branch January 3, 2023 20:47
@github-actions
Copy link
Contributor

github-actions bot commented Jan 3, 2023

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Don't override sdk version with timber ([#2450](https://github.com/getsentry/sentry-java/pull/2450))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 2f2097e

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.

Timber should only add a new package and not replace the whole sdk metadata
3 participants