-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
Updated travis ci for gradle 5 support #19
Conversation
Should you update |
.travis.yml
Outdated
- curl -s "https://get.sdkman.io" | bash | ||
- source ~/.bash_profile | ||
- sdk install kscript | ||
- sdk install gradle 5.2.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you remove the version it should take the latest one, that is the one we most care about probably if we take the brew way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i always use to use fixed versions, if there is an updated version could break something and we not have any control of that. I think is a good idea use known versions. BTW, with gradle 5 there is some problems on kotlin native.. i'm working on that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, but in this specific case, given we are going to use brew as explained in the article @orta posted here #13 (comment), brew doesn't use versions, this means that if we add gradle as dependency who installs danger-kotlin gets it with the latest version available, then I would suggest to test against the one the people will get
Generated by 🚫 Danger Kotlin against abbbcfa |
build.gradle
Outdated
@@ -1,5 +1,19 @@ | |||
buildscript { | |||
repositories { | |||
mavenCentral() | |||
maven { url 'https://dl.bintray.com/kotlin/kotlin-dev' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need two with dev and eap?
@@ -1,5 +1,5 @@ | |||
plugins { | |||
id 'org.jetbrains.kotlin.jvm' version '1.3.0' | |||
id 'org.jetbrains.kotlin.jvm' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No fixed version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed.. is defined in gradle properties
@@ -1,17 +1,17 @@ | |||
plugins { | |||
id 'org.jetbrains.kotlin.konan' version "1.3.11" | |||
id 'kotlin-multiplatform' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
import platform.posix.* | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG just few comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, the only thing is that takes more to build but we can have a look at that later.
Thank you @gianluz, this was 💯
fixes #16