Skip to content
This repository was archived by the owner on Jun 27, 2020. It is now read-only.

Conversation

@fynngodau
Copy link
Contributor

This pull request adds a free build flavor (as opposed to the new proprietary build flavor) which does not compile the Google Crashlytics dependency. For this, it utilizes the AnswersProxy class, which is now different in the different flavors and renamed to CrashlyticsProxy to better account for its purpose. In the proprietary one, it is largely unchanged (one minor change to move the code to init Crashlytics¹), in the free one, it doesn't do anything. Closes #181.

¹Not sure how exactly that piece of code works. Functionality shouldn't differ but I haven't tested it.

@fynngodau fynngodau mentioned this pull request Aug 28, 2019
@fynngodau
Copy link
Contributor Author

The pull request now also removes dropbox backup/restore feature and thus dependency on com.dropbox.core:dropbox-core-sdk:3.0.5 from the free build flavor.

@yev-kanivets yev-kanivets self-requested a review August 28, 2019 15:41
@fynngodau
Copy link
Contributor Author

Reverting the previous change as it is not necessary unlike I assumed.

Copy link

@Yky Yky left a comment

Choose a reason for hiding this comment

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

Looks good otherwise :)

*/

public class CrashlyticsProxy {
private static CrashlyticsProxy instance;
Copy link

Choose a reason for hiding this comment

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

This singleton implementation is not thread safe. Could be improved with double-checked locking.

public class AnswersProxy {
private static AnswersProxy instance;
public class CrashlyticsProxy {
private static CrashlyticsProxy instance;
Copy link

Choose a reason for hiding this comment

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

see above

@yev-kanivets yev-kanivets merged commit 17d8b1d into yev-kanivets:master Aug 29, 2019
@yev-kanivets
Copy link
Owner

@fynngodau looks great (even with a possible thread safety issue pointed out by @Yky). Thanks for your contribution 🙏

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.

Remove Crashlytics?

3 participants