[GitHub] erisu commented on a change in pull request #441: CB-14089: (android) Add Kotlin support
erisu commented on a change in pull request #441: CB-14089: (android) Add
Kotlin support
URL: https://github.com/apache/cordova-android/pull/441#discussion_r216679161
##
File path: bin/templates/project/app/build.gradle
##
@@ -30,13 +32,14 @@ buildscript {
dependencies {
classpath 'com.android.tools.build:gradle:3.1.0'
+classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
Review comment:
@joshchandler Sorry, the `cdvHelpers` is currently not available in the
`dependencies` scope, as I mentioned in this example.
Wrapping the apply plugin should be OK. Example:
```
if (cdvHelpers.getConfigPreference('EnableKotlin', 'false').toBoolean()) {
apply plugin: 'kotlin-android'
}
```
There is a discussion on a PR that also modifies the the same `build.gradle`
file for other features. It also had the same issue where `cdvHelpers` is not
available in the dependency scope. Adding it is as easy as adding the `apply
from: '../CordovaLib/cordova.gradle'` in the scope BUT I am waiting for any
feedback about it.
You can see the comment here:
https://github.com/apache/cordova-android/pull/438#discussion_r216195552
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@cordova.apache.org
For additional commands, e-mail: commits-h...@cordova.apache.org
[GitHub] erisu commented on a change in pull request #441: CB-14089: (android) Add Kotlin support
erisu commented on a change in pull request #441: CB-14089: (android) Add Kotlin support URL: https://github.com/apache/cordova-android/pull/441#discussion_r215496283 ## File path: bin/templates/project/Activity.kt ## @@ -0,0 +1,38 @@ +/* Review comment: If we make Kotlin an optional feature, this file should not be used and defaults to `Activity.java`. Only use this file if the EnableKotlin flag is set to `true`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@cordova.apache.org For additional commands, e-mail: commits-h...@cordova.apache.org
[GitHub] erisu commented on a change in pull request #441: CB-14089: (android) Add Kotlin support
erisu commented on a change in pull request #441: CB-14089: (android) Add Kotlin support URL: https://github.com/apache/cordova-android/pull/441#discussion_r215494867 ## File path: test/gradle/wrapper/gradle-wrapper.properties ## @@ -1,6 +1,5 @@ -#Wed Oct 25 11:17:25 PDT 2017 distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-4.1-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-4.4-bin.zip Review comment: I believe upgrading gradle should be in a separate PR. @dpogue Do you also agree with this? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@cordova.apache.org For additional commands, e-mail: commits-h...@cordova.apache.org
[GitHub] erisu commented on a change in pull request #441: CB-14089: (android) Add Kotlin support
erisu commented on a change in pull request #441: CB-14089: (android) Add
Kotlin support
URL: https://github.com/apache/cordova-android/pull/441#discussion_r215495327
##
File path: bin/templates/project/app/build.gradle
##
@@ -30,13 +32,14 @@ buildscript {
dependencies {
classpath 'com.android.tools.build:gradle:3.1.0'
+classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
Review comment:
Can we wrap all of the Kotlin code and make it conditional by a flag? This
is regarding to the same concerns that @dpogue mentioned.
It might be possible that not everyone would use Kotlin.
For example:
```
if (cdvHelpers.getConfigPreference('EnableKotlin', 'false').toBoolean()) {
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@cordova.apache.org
For additional commands, e-mail: commits-h...@cordova.apache.org
[GitHub] erisu commented on a change in pull request #441: CB-14089: (android) Add Kotlin support
erisu commented on a change in pull request #441: CB-14089: (android) Add
Kotlin support
URL: https://github.com/apache/cordova-android/pull/441#discussion_r215495532
##
File path: bin/templates/project/app/build.gradle
##
@@ -30,13 +32,14 @@ buildscript {
dependencies {
classpath 'com.android.tools.build:gradle:3.1.0'
+classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
Review comment:
And this conditional statement would be applied everywhere else relating to
Kotlin.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@cordova.apache.org
For additional commands, e-mail: commits-h...@cordova.apache.org
