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

Removed Crashlytics and references to maven.io #18

Merged
merged 2 commits into from Oct 8, 2020

Conversation

samtinel
Copy link

This PR removes Crashlytics and reference to maven.io. This means it is buildable by F-Droid. To be included, metadata has to be added, see #17 .

@lloydtorres
Copy link
Owner

Thanks so much for digging into this!

Per the commit that implemented Crashlytics, you'll also need to remove the app/fabric.properties file and the io.fabric.ApiKey metadata tag from the manifest. Would you mind updating your pull request with those changes?

Also, would you like to be added to the contributors list? If so, what name would you prefer for that? I'll make a separate commit to add you in once the pull request is merged in.

@samtinel
Copy link
Author

samtinel commented Oct 2, 2020

updated the PR with the new changes, but please do not merge yet, tomorrow I will test more thoroughly

Also, would you like to be added to the contributors list?

yes, thanks! use "Samtinel" please

@samtinel samtinel force-pushed the master branch 3 times, most recently from 3318d35 to fb5168a Compare October 3, 2020 07:51
@samtinel
Copy link
Author

samtinel commented Oct 3, 2020

It currently doesn't build for me, looks like gradle. will report back.
Could not initialize class org.codehaus.groovy.runtime.InvokerHelper

@samtinel
Copy link
Author

samtinel commented Oct 3, 2020

Ah, interesting. Apparently Gradle has to be >= 6.3 to work with JDK 14. Downgrading java fixed that. Building works.

@samtinel
Copy link
Author

samtinel commented Oct 3, 2020

It runs also fine through the smoke test. What I wonder is if in the settings the switch "send crash reports" also needs removal. A first look didn't really tell me what SETTING_CRASHREPORT does.
Will dig into it in the following days.

@lloydtorres
Copy link
Owner

Good catch, definitely get rid of that switch while you're at it.

@samtinel
Copy link
Author

samtinel commented Oct 5, 2020

Removed the toggle and its resources, as well as the mention of CrashLytics in the readme. It compiles and passes the smoke test.
So in my eyes this is ready to merge, and then the last hurdle is #17. After that maybe a minor version bump?

@lloydtorres
Copy link
Owner

This pull request LGTM, thanks so much for doing this!

We can handle blocking changes related to F-Droid in #17.

@lloydtorres lloydtorres merged commit d467c9f into lloydtorres:master Oct 8, 2020
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

2 participants