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

feat: update all dependencies and modernize the source base #1095

Merged
merged 28 commits into from
Oct 9, 2021
Merged

feat: update all dependencies and modernize the source base #1095

merged 28 commits into from
Oct 9, 2021

Conversation

azlekov
Copy link
Contributor

@azlekov azlekov commented Jul 2, 2021

This is first of many PRs which I want to introduce in order to get this SDK up to date.

First step in my master plan is update all dependencies, Gradle version and used plugins. Also this PR will cover updating to Java 8 and taking advantage of all sweet features of it. I prefer to make a small incremental steps so the PRs to be easy to review.

Code optimisation which have been done:

  • Replace all anonymous classes with lambdas where possible.
  • Make all fields final where possible
  • Internal classes as static where possible
  • Replace test assertions with context ones
  • Use diamond <> where possible
  • Use method references where possible
  • Imports re-arrange

NOTE: No business logic was changed so far

Updating of the Firebase Messaging should address #1071 (comment) as there introduce some breaking changes and when using the Parse FCM there are signature issues with latest version of FCM

azlekov added 2 commits July 2, 2021 19:03
- Replace all anonymous classes with lambdas where possible.
- Make all fields final where possible
- Internal classes as static where possible
- Replace test assertions with context ones
- Use diamond <> where possible
- Use method references where possible
- Imports re-arrange
@azlekov azlekov changed the title Update all dependencies Update all dependencies and modernize the source base Jul 2, 2021
@azlekov
Copy link
Contributor Author

azlekov commented Jul 2, 2021

Hello @Jawnnypoo @mtrezza I finally dared to submit my first PR here. Not sure if you are the right persons to ping, but I saw you're more likely active here. Don't be afraid of the huge changes, most of them are just replacing callbacks with lambdas and just syntax sugaring around Java 8.

I will be very happy if you have time to check this PR.

@rogerhu
Copy link
Contributor

rogerhu commented Jul 2, 2021

Thanks for the work!

@azlekov azlekov marked this pull request as ready for review July 5, 2021 10:06
Copy link
Member

@Jawnnypoo Jawnnypoo left a comment

Choose a reason for hiding this comment

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

Wow, this is a huge PR! Thanks for all the effort, I have not had time to be active with this project, but it looks good from my perspective!

@Jawnnypoo
Copy link
Member

@L3K0V Looks like there is a compilation issue:

Parse-SDK-Android/parse/src/main/java/com/parse/ParseCommandCache.java:110: error: variable context might not have been initialized

@azlekov
Copy link
Contributor Author

azlekov commented Jul 8, 2021

@Jawnnypoo any ideas what can cause this? On this line there's nothing interesting....

image

@Jawnnypoo
Copy link
Member

Hmm yeah I am not really sure. Maybe you can remove this check.

@azlekov
Copy link
Contributor Author

azlekov commented Jul 8, 2021

Fun fact is that locally I'm able to run the same Gradle script as the GitHub CI and it's working.

@mtrezza mtrezza closed this Oct 9, 2021
@mtrezza mtrezza reopened this Oct 9, 2021
@azlekov azlekov closed this Oct 9, 2021
@azlekov azlekov reopened this Oct 9, 2021
Specify kotlin and java compatibility versions all over the place
@codecov
Copy link

codecov bot commented Oct 9, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@0362b10). Click here to learn what that means.
The diff coverage is 63.03%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1095   +/-   ##
=========================================
  Coverage          ?   65.06%           
  Complexity        ?     2208           
=========================================
  Files             ?      122           
  Lines             ?     9463           
  Branches          ?     1317           
=========================================
  Hits              ?     6157           
  Misses            ?     2799           
  Partials          ?      507           
Impacted Files Coverage Δ
.../src/main/java/com/parse/CacheQueryController.java 6.81% <0.00%> (ø)
...c/main/java/com/parse/KnownParseObjectDecoder.java 50.00% <ø> (ø)
...arse/src/main/java/com/parse/LocationNotifier.java 0.00% <0.00%> (ø)
parse/src/main/java/com/parse/LockSet.java 0.00% <0.00%> (ø)
...rc/main/java/com/parse/OfflineQueryController.java 100.00% <ø> (ø)
parse/src/main/java/com/parse/ParseACL.java 89.15% <ø> (ø)
.../main/java/com/parse/ParseAnalyticsController.java 100.00% <ø> (ø)
...e/src/main/java/com/parse/ParseAnonymousUtils.java 57.14% <ø> (ø)
...rse/src/main/java/com/parse/ParseCommandCache.java 2.22% <0.00%> (ø)
...java/com/parse/ParseCountingByteArrayHttpBody.java 82.35% <ø> (ø)
... and 64 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0362b10...b9f7a89. Read the comment docs.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.codecov.yml Show resolved Hide resolved
Set minimum coverage of 65% instead of 45%
@azlekov azlekov requested review from Jawnnypoo and mtrezza October 9, 2021 21:07
@mtrezza mtrezza changed the title Update all dependencies and modernize the source base feat: update all dependencies and modernize the source base Oct 9, 2021
Copy link
Member

@mtrezza mtrezza 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! This is truly a mammoth PR, and a milestone for the Parse Android SDK!

@mtrezza mtrezza merged commit a977d11 into parse-community:master Oct 9, 2021
github-actions bot pushed a commit to mtrezza/Parse-SDK-Android that referenced this pull request Oct 10, 2021
# [2.0.0](1.22.0...2.0.0) (2021-10-10)

### Bug Fixes

* remove GCM module ([parse-community#1091](https://github.com/mtrezza/Parse-SDK-Android/issues/1091)) ([aa16bcf](aa16bcf))
* upgrade Facebook SDK ([parse-community#1105](https://github.com/mtrezza/Parse-SDK-Android/issues/1105)) ([6f4bdb0](6f4bdb0))

### Features

* update all dependencies and modernize the source base ([parse-community#1095](https://github.com/mtrezza/Parse-SDK-Android/issues/1095)) ([a977d11](a977d11))

### BREAKING CHANGES

* CHANGE

Support for Google Cloud Messaging (GCM) is removed, use Firebase Cloud Messaging instead. See the [Google developer documentation](https://developers.google.com/cloud-messaging/faq) for more details and migration assistance. ([aa16bcf](aa16bcf))
@azlekov azlekov deleted the new-hope branch October 12, 2021 17:00
@@ -45,10 +34,6 @@ artifacts {

apply plugin: 'jacoco'

jacoco {

Choose a reason for hiding this comment

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

Why removal of this? Other files still have this block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! Feel free to add it to your #1122

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.

5 participants