-
-
Notifications
You must be signed in to change notification settings - Fork 735
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
feat: update project dependencies and code refactoring #1141
feat: update project dependencies and code refactoring #1141
Conversation
Thanks for opening this pull request!
|
Codecov Report
@@ Coverage Diff @@
## master #1141 +/- ##
============================================
+ Coverage 66.27% 66.81% +0.53%
+ Complexity 2251 2249 -2
============================================
Files 122 121 -1
Lines 9983 9892 -91
Branches 1343 1332 -11
============================================
- Hits 6616 6609 -7
+ Misses 2850 2771 -79
+ Partials 517 512 -5
Continue to review full report at Codecov.
|
@mtrezza in continue of #1095 I did another batch of dependencies updates to all modules dependencies and update again everything. Unfortunately it's time to update min SDK version to 21 as we discussed in #1123 because of two reasons:
The discussion here #1123 (comment) Fortunately the upgrades went smooth so far without big changes. |
Remove deprecated code calls Refactor code and remove not needed Android SDK checks
@mtrezza again this PR is baby step ahead. I have updated the description adding the breaking changes this PR introduce. And because there was no activity on the Android SDK if you are okay I decide to clean up and remove the deprecated methods Parse Android SDK provides, so it will be less code to maintain and to migrate to Kotlin after that. |
@L3K0V you marked this as DRAFT after requesting the review. If you still request the review please remove the DRAFT flag and request again. |
Sorry @mtrezza, I found few things which I should improve soon. |
As not sure if all libraries supports 1.6.0 which was released one or two days ago
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, that's a pretty nice clean-up!
It seems this will be a major release as it will contain some breaking changes:
- minSdkVersion 16 to 21
- com.google.firebase:firebase-messaging 22.0.0 to 23.0.0 --> this does not have to mentioned explicitly, as it requires minSDK 19 which is lower than the bump to 21, but was google play services required for 22 as well?
- many deprecated public methods have been removed, they should be listed explicitly in the release notes, could you list them here in a comment, so I can use that to compose the breaking change note?
@mtrezza here you go:
In the release notes they say nothing about bumping the API level, but the release we use (the latest available) is from July, so I expect soon new versions to be released. Deprecated methods:Parse
Twitter Utils
I wonder how to compose an |
@mtrezza maybe I didn't get you right about
I think still Google Play Services are required for all versions of Firebase Messaging. In the release notes they said that:
So I expect that they still depend on it. Also here is mentioned that cloud messaing still requires Google Play Services |
Yes, I have seen that table, but I couldn't tell from it whether they only added the Google Play Services dependency with version 23 or whether Cloud Messaging was already dependent on GPS in version 22. But if you say it was dependent also in version 22 then it's not a breaking change we need to mention.
Maybe take a look at this compatibility table for inspiration, simple and it works; and you can also just add this as a headline to the README instead of creating a new .md doc. |
I'm suggesting this breaking note in the release notes, any additions/changes? Never mind the formatting, it has to be written in 1 line for the changelog auto-generator to pick it up properly. BREAKING CHANGE: The required minimum API level changes from 16 to 21. The following deprecated methods are removed: |
@mtrezza I have added some Compatibility section. It would be great if you check it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made some changes to the compatibility table and moved the API level rules into the CONTRIBUTING doc, as it's more relevant for contributors than for developers. Could you review these changes and let me know if the wording is fine? Then we should be good to merge.
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me too!
# [2.2.0](2.1.0...2.2.0) (2021-11-25) ### Features * update project dependencies and code refactoring ([#1141](#1141)) ([38d2ebb](38d2ebb))
New Pull Request Checklist
Issue Description
The project should be up-to-date with all dependencies, Gradle version, Android Studio etc but is not.
It uses an old versions of the integration libraries like Firebase, Facebook and also the Parse Android SDK is stuck on very old version of OkHttp which does not support newer TLS versions. Updating OkHttp requires bump of min Android SDK version to API 21. Also the new Firebase libraries also bump their min version to API 19
Also some of the modules uses deprecated APIs from Bolts and other libraries. The idea is to update the source base a bit.
Related issue: #1142 #1100 #1123 #1095
Closes: #1142
Approach
The approach here will be to update all dependencies, Gradle wrapper version and AGP to the latest versions. Also cleaning up all deprecated code calls and replace them with their new options or variants. Because there was no activity and new Parse Android SDK versions lately, it's save to remove all deprecated Parse APIs.
Breaking changes
Parse.getParseDir()
TODOs before merging
Add testsAdd changes to documentation (guides, repository pages, in-code descriptions)