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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add auto-installation for sentry-android SDK and integrations #282

Merged
merged 18 commits into from Mar 9, 2022

Conversation

romtsn
Copy link
Member

@romtsn romtsn commented Feb 16, 2022

#skip-changelog

馃摐 Description

  • Adds auto-installation for sentry-android and timber, okhttp and fragment integrations
  • We perform existing dependencies/version checks only for the direct (first-level) dependencies, because the mechanisms Gradle provides do not allow calculating the entire dependency graph at this stage. There's a way to resolve the entire graph (copying a configuration and resolving it), but this is very costly to do at configuration phase, so I'd really not wanna do that. I think this is best effort we do now, and it will cover most of the cases.
  • Because of the above I've added an explicit sentryVersion configuration setting, so the users can specify the version, for instance, if they use an older sentry-android version that is coming as a transitive dependency from one of their submodules/libs.

This is what it looks like in practice:
image
Notice how there are no sentry dependencies in the SS above.
When we build the app, the final apk looks like this (with all integrations and the SDK included):
image

馃挕 Motivation and Context

Q1 goal

馃挌 How did you test it?

馃摑 Checklist

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

馃敭 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2022

Messages
馃摉 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 馃毇 dangerJS against a44bf09

@romtsn romtsn marked this pull request as ready for review February 18, 2022 20:32
@romtsn romtsn deleted the branch feat/auto-installation February 18, 2022 20:41
@romtsn romtsn closed this Feb 18, 2022
@romtsn romtsn reopened this Feb 18, 2022
@romtsn romtsn changed the base branch from feat/auto-installation to main February 18, 2022 20:42
@romtsn romtsn changed the base branch from main to feat/auto-installation February 18, 2022 20:42
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Maybe I'm wrong to suggest was there instead of is. Or perhaps we should log: installing ayz package since I believe at that point we're just adding it to the list of dependencies.

Perhaps even: Adding package xyz to the dependencies list

@romtsn
Copy link
Member Author

romtsn commented Feb 22, 2022

Had to revert back to using singleton to share AutoInstallState, cus build services cannot be serialized (and the ComponentMetadataRules are serialized). Did thorough testing - no problems occurred.

@romtsn
Copy link
Member Author

romtsn commented Mar 7, 2022

@marandaneto addressed the comments, you could take another look if you will.

@romtsn romtsn merged commit 0fae1e9 into feat/auto-installation Mar 9, 2022
@romtsn romtsn deleted the feat/add-dependencies branch March 9, 2022 10:24
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.

None yet

4 participants