Updated kotlin example #1928
Updated kotlin example #1928
Conversation
|
@WeaponMan thanks for updating the sample. Can you remove the non-working Pref from this PR? I want to merge this, but we should not commit non-working example code. We want to send the non-working code to kotlin devs ofc. |
|
also i'm not sure if we want to use kotlin EAP in the sample project? |
|
I agree, we should only use stable versions. Why did you update to EAP?
Again, it is ok in the test branch which we will show to Kotlin devs.
…On Dec 30, 2016 16:03, "Kay-Uwe Janssen" ***@***.***> wrote:
also i'm not sure if we want to use kotlin EAP in the sample project?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1928 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAdU5Y48ESQWazP6swWP6O9a9J28YoWnks5rNR1YgaJpZM4LYKt8>
.
|
|
I updated to EAP, because kapt2 was unable to find AndroidManifest file. I will revert non working stuff and remove eap version from this PR tomorrow. |
|
Updated to 1.0.6 which was released on Dec 27. Totally missed that. |
| @@ -1,10 +1,11 @@ | |||
| buildscript { | |||
| ext.kotlin_version = '1.0.6' | |||
|
|
||
| @EActivity(R.layout.main) | ||
| open public class HelloAndroidActivity : Activity() { | ||
| open class HelloAndroidActivity : Activity() { |
There was a problem hiding this comment.
Classes with no access modifier are always public in kotlin. There is no need to type public.
https://kotlinlang.org/docs/reference/visibility-modifiers.html
|
@WeaponMan pls next time use a topic branch, not just |
WonderCsabo
left a comment
There was a problem hiding this comment.
Please squash the commits.
|
Squashed into one commit. |
| classpath 'com.android.tools.build:gradle:2.1.0' | ||
| classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.0.2' | ||
| classpath 'com.android.tools.build:gradle:2.2.3' | ||
| classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" |
There was a problem hiding this comment.
This is needed because of string interpolation.
There was a problem hiding this comment.
okay. do we want to be consistent and update the other lines too? if it is no noticeable performance drawback?
There was a problem hiding this comment.
I think we should use double quotes in all Gradle scripts for strings, yes. It looks better for consistency.
| dependencies { | ||
| classpath 'com.android.tools.build:gradle:2.1.0' | ||
| classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.0.2' | ||
| classpath 'com.android.tools.build:gradle:2.2.3' |
There was a problem hiding this comment.
when updating versions in this sample we should also update the versions in the gradle sample (and if necessary in the maven samples too).
| android { | ||
| compileSdkVersion 23 | ||
| buildToolsVersion "23.0.3" | ||
| compileSdkVersion 25 |
There was a problem hiding this comment.
when updating versions in this sample we should also update the versions in the gradle sample (and if necessary in the maven samples too).
|
Thank you for your contribution @WeaponMan . I added some additional comments. Unfortunately the PR is already merged. @WonderCsabo could you have a look at those comments? i think at least for the updated versions in the other samples we should have another PR. |
|
Yes, we should update all samples (include the Maven one), to use latest and greatest plugin and dependency versions. |
|
Added new issue for updating the samples #1932 |
I updated to last eap version of kotlin, because 1.0.5-3 with kapt2 is unable to find AndroidManifest.
I added shared preferences example that is not working with kotlin.