Apps-android-commons: Update libraries version to the latest stable version.

[knight-shade]

Summary:

The project is using old version of libraries, for which already an update version have been released.
Following libraries are outdated:

• Gson from 2.8.1 to 2.8.2

• okHttp from 3.8.1 to 3.9.1

• Fresco from 1.3.0 to 1.5.0

• Leakcanary from 1.5.1 to 1.5.4

• Kotlin from 1.1.15 to 1.2.30

Should be also update buildToolsVersion and compileSdkVersion to 27?

[maskaravivek]

@knight-shade Can you briefly add a description about why each of these libraries needs to be updated? :)

[nicolas-raoul]

I think that a reason would only be needed for NOT updating a library :-)

2018/03/07 15:39 "Vivek Maskara" [e-mail]:

@knight-shade https://github.com/knight-shade Can you briefly add a
description about why each of these libraries needs to be updated? :)

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/commons-app/apps-android-commons/issues/1268#issuecomment-371039066,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGFBkT85V3W6i4lk8aS_NiAu-x0swsXks5tb4CwgaJpZM4Sf6MX
.

[misaochan]

I'm good with updating libraries (alongside buildToolsVersion etc), with the caveat that they need to be tested on a Release build variant (preferably ProdRelease) before merging. We have unfortunately had issues with library changes causing issues in a Release build but not a Debug build. :/

So, if anyone is submitting a PR for this, please test it on a Release build variant and mention in your PR the tests you did and their details (what build variant, which parts of the app you tested, which API).

[tanvidadu]

We can also have commit per update as it might help in testing and isolation of possible errors, which else would be difficult to resolve.
I will like to work on it but i might need help while testing it on Release Build Variant

[knight-shade]

@tanvidadu I am already working on the issue.

[maskaravivek]

My concern at updating all the libraries at once was exactly what @misaochan is pointing to. Extensive testing needs to be done if we update the libraries.

Moreover unless updating the library fixes some issue that our app is currently facing or introduces a feature that we might be interested in using, this exercise will only add testing efforts from our side. :)

On the other hand I completely agree with keeping buildToolsVersion etc up to date. These updates come from Google and thus give much more confidence while updating.

[knight-shade]

@maskaravivek Yup, indeed it would be intense testing exercise. Ya, the app is not facing any issues with current version of libraries so there isn't any strong backing factor for upgrading it. Everyone is good here regarding buildToolsVersion etc up to date. Since kotlin is being used and its backed by jetbrains what are your opinions on upgrading it?

[psh]

So, the TL;DR version - I think we might want to update Fresco but the other libraries can probably wait

The longer version:

• Kotlin is currently only being used in tests and isnt enabled for the core app codebase. In that regard I dont think it really matters if it lags a little behind the latest version ... I dont see a lot of people writing tests for the project right now, nor people converting the Java tests over.

• I took a look at upgrading OkHttp this evening - the process was smooth but again, this isnt a library that is in wide use in the application as of yet; its a transitive dependency that was called out as the future direction of where we want to go with removing obsolete networking code. Looking through the release notes for OkHttp there arent any changes that form a compelling reason to upgrade.

• We are actually a lot further behind the Fresco latest version than I expected, or this issue lead me to believe. We are using a single class from the library (the SimpleDraweeView) and that's core to all of the image viewing in our app. If anything should be the focus out of this list I'd look at this library out of all of them.

• The release notes for Gson were vague - just talking about "bug fixes" - not sure if that really constitutes a compelling reason to move the version given such a minor version bump. When we move to the JSON based Wikimedia API, Gson will become a much more important part of the picture and we should probably wrap its upgrade into testing of the new API layer.

• Leak Canary has some improvements but it only really benefits developers - we dont have it enabled in the production released app - probably safe to update as it brings some better reporting, assuming anyone really looks for memory leaks during development... ;)

[knight-shade]

@maskaravivek @misaochan Thank you @psh for a detailed report, Everybody knows this , although just wanted to make it little bit more explicit, upgrading buildToolsVersion ,etc will also update the support libraries and Leak Canary, Fresco needs to be updated.

[misaochan]

@knight-shade Sounds like a good plan, I would go with @maskaravivek and @psh 's suggestion of upgrading buildToolsVersion and Fresco. Please test this on Release build variant and provide the details (what features you tested) in your PR. You will likely need to create your own personal keys to use for Release build testing. :)

[knight-shade]

@misaochan By release build variant, you meant betaRelease or prodRelease ? I am currently testing with betaRelease.

Update

Upgraded:
fresco to 1.5.0
leak-canary to 1.5.4
support library version to 27.1.0
compileSdkVersion to 27
buildToolsVersion to 27
targetSdkVersion to 27

New issue after these update

1. App crashes after clicking on images uploaded by user.
2. 5 test failing in betaDebug due to above crash.
3. Many many more testing failing in betaRelease version. :worried:

Update

Now, only 5 test failing to due issue number 1 (mentioned above) on both versions of beta build variant.

Update

• There is a bug in support library version 27.1.0, reported and fixed here, which was the root cause for app crash. It will be released in upcoming update of Support library. So support library version as of now is 27.0.2.

• Below are the libraries then have been updated and all tests in betaDebug and betaRelease are successful.
  fresco to 1.5.0
  leak-canary to 1.5.4
  support library version to 27.1.0
  compileSdkVersion to 27
  buildToolsVersion to 27
  targetSdkVersion to 27
  multidex to 1.0.3

• I have tested all features of app by manually exploring , and all of them work as intended.

@misaochan Should I make a PR now?

[maskaravivek]

@knight-shade Awesome work. :)

Yes, please go ahead and create a PR if you have managed to resolve all the issues around updating the libraries.

[misaochan]

Thanks @knight-shade ! :)

betaRelease is fine this time, but in general wherever possible, testing on prodRelease is best. The beta variant is just there to assist in cases where many files that are "unsuitable" for Commons need to be uploaded for test purposes, so that we don't spam Commons with useless files. The prod variant is the variant that users will get.

[knight-shade]

Thank you @misaochan much appreciated, yup will keep that in mind for next time. :smile: Guys @maskaravivek @misaochan all the tests pass on my PC, but the build fails on travis CI build is failing, and I am totally out of ideas and stackoverflow why it is happening. Can someone lend me a helping hand?

Analysing travis CI log points that -> Failed to find Build Tools revision 27.0.2
Just checked here, its a valid version.

[misaochan]

Pinging @whym for the Travis Q.

[whym]

re Travis: we need something like c11db21fc95fc for 'android-27', I believe. (And maybe that will make older entries redundant?) Sorry I mixed it up. For build tools, I believe the previous change was in 1800c544558ed7c7ff72b4de67034dbb65d8e002.

[dbrant]

Closing in favor of newer task #2591