Refactor Build System with Kotlin DSL using pre-compiled scripts. #5

android10 · 2020-06-10T14:52:26Z

This PR aims to refactor our gradle build system in order to organize it by functionality in a more modular way using Kotlin DSL pre-compiled scripts.

Includes:

Basic Build Variants Setup.
Quality: which includes Lint Configuration.
Compilation Options.
Minor Refactor.

Everything has been extracted In order to keep the main build.gradle.kts file small and easy to understand.

…ompiled script.

Gardosen · 2020-06-10T16:01:17Z

buildSrc/build.gradle.kts

-    jcenter()
-    google()
+private enum class DEPENDENCIES(val value: String) {
+    AndroidBuildTools("com.android.tools.build:gradle:4.0.0")


wouldn't it make more sense to use string values class instead of an enum? this way the calls would DEPENDENCIES.AndroidBuildTools, instead of having to add .value behind it.

@Gardosen Enums create scoping and group values. I do not see any reason to use strings constants...plus it forces the client to use predefined values that belong to an specific scope.

I don't know if we can use @StringDef at this level (guess it was an android thing), but it might be a good alternative. This looks a bit verbose for me too. I think enum here does not impose any level of strictness, since there's nothing that prevents us from putting any other String inside implementation block. why did you prefer enums here over sth like below? (which is also consistent with how we declare dependencies in app module)

object Dependencies { const val ANDROID_BUILD_TOOLS = "com.android..." } implementation(Dependencies.ANDROID_BUILD_TOOLS)

We cannot use @StringDef or I would not like to add the dependency just for having this. The Object does the job and keeps consistency. I will modify this.

Gardosen · 2020-06-10T16:03:53Z

buildSrc/src/main/kotlin/scripts/variants.gradle.kts

+    DEV("dev"), INTERNAL("internal"), PUBLIC("public")
+}
+
+private enum class FlavorDimensions(val value: String) {


FlavorDimensions for the version? do we have a better name for this?

@Gardosen do you have a suggestion? Please also suggest how you would do it instead. That is what we are expecting from code reviews :). Otherwise that would be a complain. :)

A version is a version here in this case, I do not see any other way of describing it. 🤔

FlavorDimensions is a gradle thing, so I think the name is ok. However, the name "version" is a bit confusing for me. Why did we choose that name? How does versioning contribute to dimensions?

Dimension is use to group flavors. You can group flavors based on different contexts/dimensions. In this case I did not find a better way of grouping this. Suggestions?

If we were to have for example these flavors:

Premium

Pro

Semi-Pro

Then dimension would be payments, because they are based/belong to this area.

I usually put "default" as the name, if I have a single dimension

default does not tell me much. And I'm also considering to extend this with other dimensions related to "customer" which are custom builds.

Flavor Dimensions allow us to define groups of flavors. Gradle creates every combination between these groups for us. No need to mix and match by hand. That is why I find default a bit senseless. In any case, I want to merge this so I will get back to it. It is already modified.

mejdoo · 2020-06-11T11:44:51Z

buildSrc/src/main/kotlin/scripts/variants.gradle.kts

+            applicationIdSuffix = ".${ProductFlavors.INTERNAL.value}"
+            versionNameSuffix = "-${ProductFlavors.INTERNAL.value}"
+        }
+        create(ProductFlavors.PUBLIC.value) {


I think ProductFlavors.PRODmakes more sense as a name convention for this flavor

I considered that, but the reason why not is that for the time being we will have this for the public, but there will be other product flavors that are specific for customers.

From my perspective: Release means Production, then you have different product flavors that satisfy different contexts. Does that make sense? How would you organize this then for custom builds for customers?

I don't think the term release means production in this context. In our case, we have some combination between flavors/build types like devRelease and internalRelease but they don't have any relation with the production version except enabling proguard.

Anyway, it makes sense to use term PUBLIC if we wanna add more flavors for our customers.

Good point about the definition of release. And related to Public it is exactly what you pointed out. Thanks for the feedback.

buildSrc/src/main/kotlin/core/android.gradle.kts

app/build.gradle.kts

gizemb · 2020-06-11T11:33:21Z

buildSrc/build.gradle.kts

-    jcenter()
-    google()
+private enum class DEPENDENCIES(val value: String) {
+    AndroidBuildTools("com.android.tools.build:gradle:4.0.0")


I don't know if we can use @StringDef at this level (guess it was an android thing), but it might be a good alternative. This looks a bit verbose for me too. I think enum here does not impose any level of strictness, since there's nothing that prevents us from putting any other String inside implementation block. why did you prefer enums here over sth like below? (which is also consistent with how we declare dependencies in app module)

object Dependencies { const val ANDROID_BUILD_TOOLS = "com.android..." } implementation(Dependencies.ANDROID_BUILD_TOOLS)

gizemb · 2020-06-11T11:51:30Z

buildSrc/src/main/kotlin/scripts/variants.gradle.kts

+    DEV("dev"), INTERNAL("internal"), PUBLIC("public")
+}
+
+private enum class FlavorDimensions(val value: String) {


FlavorDimensions is a gradle thing, so I think the name is ok. However, the name "version" is a bit confusing for me. Why did we choose that name? How does versioning contribute to dimensions?

buildSrc/src/main/kotlin/scripts/variants.gradle.kts

android10

@mejdoo @gizemb I addressed your comments plus:

I have changed the flavor dimension to default. Let's define this later when we need it.
I got rid of enums in favor of objects for code readability and consistency.
Minor refactor

* Inject dispatchers for testing purposes (#58)

… time (#78)

android10 added 7 commits June 10, 2020 11:19

Define build variants.

6f4ab6e

Extract build variants to a separated pre-compiled script.

dbabada

Add enums for build variants.

4d3503d

Add enum constants for buildSrc project dependencies.

ceec85b

Extract lint options to quality pre-compiled script.

c45d66a

Extract compile and packaging options to compilation.gradle.kts pre-c…

90b36eb

…ompiled script.

Extract compile and packaging options to compilation.gradle.kts pre-c…

4501c94

…ompiled script.

android10 added the enhancement New feature or request label Jun 10, 2020

android10 requested review from Gardosen, gizemb, makingthematrix, mejdoo and BradleyIW June 10, 2020 14:52

Gardosen reviewed Jun 10, 2020

View reviewed changes

mejdoo suggested changes Jun 11, 2020

View reviewed changes

gizemb reviewed Jun 11, 2020

View reviewed changes

android10 requested review from mejdoo and Gardosen June 11, 2020 12:20

android10 added 2 commits June 11, 2020 14:30

Get rid of enums and replace them with object to keep consistency.

1aa0c99

Change flavor dimension to default.

0b6c01c

android10 commented Jun 11, 2020

View reviewed changes

android10 requested a review from gizemb June 11, 2020 13:41

mejdoo approved these changes Jun 11, 2020

View reviewed changes

gizemb approved these changes Jun 11, 2020

View reviewed changes

android10 merged commit 90068eb into master Jun 11, 2020

android10 deleted the feature-product-flavors branch June 11, 2020 13:59

gizemb added a commit that referenced this pull request Jun 17, 2020

Registration #5: Add e-mail entry layout (#11)

99cb910

gizemb added a commit that referenced this pull request Sep 3, 2020

Feat: Login #5 - Add use case for login with e-mail (#53)

04b89fb

* Inject dispatchers for testing purposes (#58)

gizemb added a commit that referenced this pull request Sep 23, 2020

Feat: User Session #5 - Ensure at most one current session at a given…

b38dfde

… time (#78)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Build System with Kotlin DSL using pre-compiled scripts. #5

Refactor Build System with Kotlin DSL using pre-compiled scripts. #5

android10 commented Jun 10, 2020

Gardosen Jun 10, 2020

android10 Jun 11, 2020

gizemb Jun 11, 2020

android10 Jun 11, 2020

Gardosen Jun 10, 2020

android10 Jun 11, 2020

gizemb Jun 11, 2020

android10 Jun 11, 2020

gizemb Jun 11, 2020

android10 Jun 11, 2020

android10 Jun 11, 2020

mejdoo Jun 11, 2020

android10 Jun 11, 2020

mejdoo Jun 11, 2020 •

edited

android10 Jun 11, 2020

gizemb Jun 11, 2020

gizemb Jun 11, 2020

android10 left a comment

Refactor Build System with Kotlin DSL using pre-compiled scripts. #5

Refactor Build System with Kotlin DSL using pre-compiled scripts. #5

Conversation

android10 commented Jun 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mejdoo Jun 11, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

android10 left a comment

Choose a reason for hiding this comment

mejdoo Jun 11, 2020 •

edited