Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

Move build scripts to Kotlin #423

Merged
merged 3 commits into from
Sep 7, 2022

Conversation

martinbonnin
Copy link
Contributor

This is a follow up from #398

This PR converts build scripts to Kotlin. I tried as much as possible to preserve the existing behaviour even if it's not really idiomatic at the moment.
This PR also bumps the Gradle version to 6.9. This is the last version that ships with the "maven" plugin and still supports Kotlin DSL.

Follow ups:

  • Migrate "maven" to "maven-publish"
  • Make the build more idiomatic

build.gradle.kts Outdated Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
@elefeint
Copy link
Contributor

@jamesward Would you like to help review this PR?

@vlsi
Copy link
Contributor

vlsi commented Jul 30, 2022

@martinbonnin , you might want to add an intermediate commit that renames build.gradle -> build.gradle.kts to help Git for displaying the history: https://stackoverflow.com/questions/44310285/converting-file-tracked-with-git-from-java-to-kotlin-in-android-studio/44320452#44320452

@martinbonnin
Copy link
Contributor Author

martinbonnin commented Jul 30, 2022

you might want to add an intermediate commit that renames build.gradle -> build.gradle.kts

Good call 👍 . I just did. Also makes it easier to review the diff with the Groovy version from that commit

Copy link
Contributor

@elefeint elefeint left a comment

Choose a reason for hiding this comment

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

I apologize for the delays.
Could you rebase this off latest?

@martinbonnin
Copy link
Contributor Author

martinbonnin commented Sep 6, 2022

Could you rebase this off latest?

It should be good now

Copy link
Contributor

@elefeint elefeint left a comment

Choose a reason for hiding this comment

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

Thank you!

build.gradle.kts Outdated Show resolved Hide resolved
@elefeint elefeint merged commit e2a00a6 into GoogleCloudPlatform:master Sep 7, 2022
This was referenced Sep 7, 2022
tagTemplate = 'v$version'
git {
requireBranch = /^release-v\d+.*$/ //regex
tagTemplate = "v$version"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have been "v\$version"

Copy link
Contributor

Choose a reason for hiding this comment

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

In #437

attributes(
mapOf(
"Implementation-Title" to project.name,
"Implementation-Version" to version,
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have been project.version rather than version from a jar task (there's a version property in jar task)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll add it to #437; thanks!

@elefeint
Copy link
Contributor

elefeint commented Sep 7, 2022

I verified that releasing process still works and the staged artifacts look good (I've dropped them, so no new release right now). Thanks, everyone!

@vlsi
Copy link
Contributor

vlsi commented Sep 11, 2022

@elefeint , I see you squashed the commits before merging. It causes broken history since Git can't detect that build.gradle -> build.gradle.kts was rename+modifications.

The intermediate commit was vital for proper git history, see my comment in #423 (comment).

Would you please clarify if "rebase and merge" (with or without merge commit) was an option at all?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants