Issue 29596603: Issue 5981 - Adjust build.gradle to support Kotlin (Closed)
|
Created:
Nov. 3, 2017, 10:54 a.m. by jens Modified:
Nov. 9, 2017, 8:27 a.m. CC:
Felix Dahlke, René Jeschke Visibility:
Public. |
DescriptionExtended build.gradle files to be able to use Kotlin for tests
Patch Set 1 : #
Total comments: 3
Patch Set 2 : Changed kotlinVersion to camel case #
MessagesTotal messages: 9
https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle File build.gradle (right): https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle#newcode2 build.gradle:2: ext.kotlin_version = '1.1.51' should not it be in the section below where the other versions are set?
https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle File build.gradle (right): https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle#newcode2 build.gradle:2: ext.kotlin_version = '1.1.51' On 2017/11/03 12:00:20, anton wrote: > should not it be in the section below where the other versions are set? Yeah, I wonder why it's not. Looks like Android Studio moved it. I'll submit a new patch set.
On 2017/11/03 12:45:54, jens wrote: > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle > File build.gradle (right): > > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle#newcode2 > build.gradle:2: ext.kotlin_version = '1.1.51' > On 2017/11/03 12:00:20, anton wrote: > > should not it be in the section below where the other versions are set? > > Yeah, I wonder why it's not. Looks like Android Studio moved it. I'll submit a > new patch set. Ah, now I know why it was moved: The buildscript block is evaluated at the very beginning, before any other part of the groovy script. Hence, properties defined in the ext block don't exist at that time.
On 2017/11/03 12:51:25, jens wrote: > On 2017/11/03 12:45:54, jens wrote: > > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle > > File build.gradle (right): > > > > > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle#newcode2 > > build.gradle:2: ext.kotlin_version = '1.1.51' > > On 2017/11/03 12:00:20, anton wrote: > > > should not it be in the section below where the other versions are set? > > > > Yeah, I wonder why it's not. Looks like Android Studio moved it. I'll submit a > > new patch set. > > Ah, now I know why it was moved: The buildscript block is evaluated at the very > beginning, before any other part of the groovy script. Hence, properties defined > in the ext block don't exist at that time. LGTM
https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle File build.gradle (right): https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle#newcode2 build.gradle:2: ext.kotlin_version = '1.1.51' On 2017/11/03 12:45:54, jens wrote: > On 2017/11/03 12:00:20, anton wrote: > > should not it be in the section below where the other versions are set? > > Yeah, I wonder why it's not. Looks like Android Studio moved it. I'll submit a > new patch set. Minor thing, but for the other variables in ext, we specify them using camelCase instead of underscore for word delimiters
On 2017/11/08 11:37:20, diegocarloslima wrote: > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle > File build.gradle (right): > > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle#newcode2 > build.gradle:2: ext.kotlin_version = '1.1.51' > On 2017/11/03 12:45:54, jens wrote: > > On 2017/11/03 12:00:20, anton wrote: > > > should not it be in the section below where the other versions are set? > > > > Yeah, I wonder why it's not. Looks like Android Studio moved it. I'll submit a > > new patch set. > > Minor thing, but for the other variables in ext, we specify them using camelCase > instead of underscore for word delimiters Uploaded a new patch set
On 2017/11/08 16:15:31, jens wrote: > On 2017/11/08 11:37:20, diegocarloslima wrote: > > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle > > File build.gradle (right): > > > > > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle#newcode2 > > build.gradle:2: ext.kotlin_version = '1.1.51' > > On 2017/11/03 12:45:54, jens wrote: > > > On 2017/11/03 12:00:20, anton wrote: > > > > should not it be in the section below where the other versions are set? > > > > > > Yeah, I wonder why it's not. Looks like Android Studio moved it. I'll submit > a > > > new patch set. > > > > Minor thing, but for the other variables in ext, we specify them using > camelCase > > instead of underscore for word delimiters > > Uploaded a new patch set LGTM
On 2017/11/08 16:15:31, jens wrote: > On 2017/11/08 11:37:20, diegocarloslima wrote: > > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle > > File build.gradle (right): > > > > > https://codereview.adblockplus.org/29596603/diff/29596614/build.gradle#newcode2 > > build.gradle:2: ext.kotlin_version = '1.1.51' > > On 2017/11/03 12:45:54, jens wrote: > > > On 2017/11/03 12:00:20, anton wrote: > > > > should not it be in the section below where the other versions are set? > > > > > > Yeah, I wonder why it's not. Looks like Android Studio moved it. I'll submit > a > > > new patch set. > > > > Minor thing, but for the other variables in ext, we specify them using > camelCase > > instead of underscore for word delimiters > > Uploaded a new patch set LGTM 
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
