Skip to content
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

Publish snapshots build against kotlin dev versions. #809

Merged
merged 15 commits into from
Aug 5, 2020

Conversation

Tapchicoma
Copy link
Collaborator

@Tapchicoma Tapchicoma commented Jul 9, 2020

Adds support for -PkotlinDev flag. This flag enables building ktlint against Kotlin development versions.

Additionally allows to publish this project snapshots that are built against dev versions. To be able to do that have to migrate publishing from 'com.vanniktech.maven.publish' plugin to generic Gradle 'maven-publish'.

Kotlin release and dev versions should be updated now in two places - build.gradle and settings.gradle. Will simplify this in the future.

build.gradle Outdated
@@ -23,6 +22,10 @@ ext.deps = [
'jimfs' : 'com.google.jimfs:jimfs:1.1'
]

if (gradle.ext.isKotlinDev) {
project.properties.put("VERSION_NAME", "0.38.0-kotlin-dev-SNAPSHOT")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the version number be dynamic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-SNAPSHOT means it will be dynamic

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tapchicoma agree here. Can we pull the main version name from the gradle.properties file or something? Otherwise we have to remember to update 0.38.0 in 2 places when we next release.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, missed here main point. Will update code to pull it from gradle.properties

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed via 8f4fa13

@Tapchicoma Tapchicoma force-pushed the publish-dev-kotlin-snapshots branch from 4b18702 to 050f9e5 Compare July 21, 2020 08:42
This allows to publish snapshots build against kotlin dev version.

Signed-off-by: Yahor Berdnikau <[email protected]>
Signed-off-by: Yahor Berdnikau <[email protected]>
This repository is not needed, 'maven-publish' actually adds it automatically.

Signed-off-by: Yahor Berdnikau <[email protected]>
Copy link
Collaborator

@romtsn romtsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I would also add a few words in readme about its existence

}
}

// Pass '-PkotlinDev' to command line to enable kotlin-in-development version
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will you also update CI script to publish with kotlinDev on master?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already done, please check latest changes.

build.gradle Outdated

if (gradle.ext.isKotlinDev) {
maven { url 'https://dl.bintray.com/kotlin/kotlin-eap' }
maven { url 'https://kotlin.bintray.com/kotlinx' }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need kotlinx repo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems added while I was testing building against dev version and forgot to remove. Removed in 3a8a398

build.gradle Outdated
@@ -23,6 +22,12 @@ ext.deps = [
'jimfs' : 'com.google.jimfs:jimfs:1.1'
]

if (gradle.ext.isKotlinDev) {
allprojects { p ->
p.ext."VERSION_NAME" = "0.38.0-kotlin-dev-SNAPSHOT"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would be also in favour of having kotlin version in the version name in case of a snapshot. Otherwise you'd always need to look up in this file, which kotlin version has been used to build the snapshot. Or maybe we could have a badge in readme to show the kotlin-dev version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to not add it in version name, as you need always check if it is changed.

Badge could be an option and definitely changelog updates.

Signed-off-by: Yahor Berdnikau <[email protected]>
Signed-off-by: Yahor Berdnikau <[email protected]>
Signed-off-by: Yahor Berdnikau <[email protected]>
It will use proper dependency on maven publish tasks.

Signed-off-by: Yahor Berdnikau <[email protected]>
Signed-off-by: Yahor Berdnikau <[email protected]>
It is a bug in Dokka, see Kotlin/dokka#294

Signed-off-by: Yahor Berdnikau <[email protected]>
@Tapchicoma Tapchicoma marked this pull request as ready for review August 4, 2020 09:16
@Tapchicoma Tapchicoma requested a review from shashachu August 4, 2020 09:41
@romtsn romtsn self-requested a review August 4, 2020 16:11
@Tapchicoma Tapchicoma merged commit 9a9b8e0 into pinterest:master Aug 5, 2020
@Tapchicoma Tapchicoma deleted the publish-dev-kotlin-snapshots branch August 5, 2020 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants