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

Update gradle dependencies for October 2018 #311

Merged
merged 10 commits into from
Oct 29, 2018

Conversation

Cliabhach
Copy link
Contributor

This is the first in what will hopefully be a series of regular maintenance changesets. This deals primarily with standardizing the 27 build.gradle files currently included in the repository.

While the functionality of output APKs should not be different, with any luck these changes will shed some light onto our ongoing build instability. Specific fixes for that issue are being developed at #285.

Included in this PR:

  1. Update Kotlin Libs from JDK 7 variant to JDK 8 variant
  2. Remove duplicate apply plugin: X lines from 26 build.gradle files
    • Here's a breakdown of the duplicates:
      • com.android.library (x1)
      • kotlin-android (x2)
      • kotlin-android-extensions (x24)
      • kotlin-kapt (x25)
    • Of these, only kotlin-android-extensions was not applied by the root build.gradle, so that had to be done as well
  3. Use the same sorting logic for all of the dependencies blocks, with three primary goals
    1. Make it easier to see what each module is actually bringing in
    2. Reduce the chance of merge conflicts if two different contributors need to modify the same gradle configs
    3. Call out frequently-used modules for inclusion in MODULES.md

build.gradle Outdated
@@ -106,6 +106,10 @@ subprojects {
apply plugin: 'com.android.library'
}
apply plugin: 'kotlin-android'
if (name != 'analytics' && name != 'image') {
// Most modules make use of the synthetic accessors; ':db' uses the experimental 'Parcelize' annotation, so it does some configuration beyond that.
apply plugin: 'kotlin-android-extensions'
Copy link
Contributor

Choose a reason for hiding this comment

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

i know these two don't need it but is there harm in applying them? looking at this from a simplification stand point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Slightly, yes. Since I'm

  1. trying to avoid changing functionality here
  2. still not confident I know precisely what the plugin does

I scheduled that for a subsequent PR. For a point of reference, the first time you apply the kapt plugin to a module you add at least .5 seconds to the configuration time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...you're not going to take no for an answer, are you?

@Cliabhach
Copy link
Contributor Author

I'll rebase on dev and add a few commits I forgot to push up.

@Cliabhach Cliabhach force-pushed the pc-update-gradle-dependencies-october-2018 branch from 3ab43aa to 471f10a Compare October 27, 2018 23:16
As it stands, 24 out of 27 build.gradle files apply that.
This reduces most gradle files to just a list of dependencies
This moves all such statements in search/build.gradle to be in
a single block.
This also splits the `implementation` and `api` statements in
the ui/build.gradle file.
In addition to sorting those entries in welcome/build.gradle, we now
only have one `implementation libs.constraint_layout` declaration.
This intends to put more emphasis on the
`implementation project(':base')` line in navigation/build.gradle.
@Cliabhach Cliabhach force-pushed the pc-update-gradle-dependencies-october-2018 branch from 471f10a to f4ba0df Compare October 29, 2018 18:53
@sam33rdhakal sam33rdhakal merged commit 8aff83a into dev Oct 29, 2018
@sam33rdhakal sam33rdhakal deleted the pc-update-gradle-dependencies-october-2018 branch October 29, 2018 20:41
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.

3 participants