-
Notifications
You must be signed in to change notification settings - Fork 13
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
CI improvements #108
CI improvements #108
Conversation
71aa716
to
41d6610
Compare
02f4f9d
to
81b542e
Compare
Update gradle to the latest version, build in CI with the latest LTS Java version, and remove unused Maven repository for generated SDKs (code generation has moved to happen in this project directly). Update to use buf maven artifact instead of an expensive (uncached) build each CI run.
81b542e
to
97cd5ce
Compare
~/go/pkg/mod | ||
key: ${{ runner.os }}-gomod-ci-${{ hashFiles('gradle.properties', 'gradle/libs.versions.toml') }} | ||
restore-keys: | ||
${{ runner.os }}-gomod-ci- |
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.
For the ci/release workflows, we now cache go modules and build for building the license header binary. We no longer need to build the buf binary as we pull it from the Maven artifact.
- uses: actions/setup-java@v4 | ||
with: | ||
distribution: 'temurin' | ||
java-version: '17' | ||
java-version: '21' |
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.
Bumping to latest LTS - we still build with source/target/release set to Java 8 compatibility.
~/go/pkg/mod | ||
key: ${{ runner.os }}-gomod-conformance-${{ hashFiles('gradle.properties', 'gradle/libs.versions.toml') }} | ||
restore-keys: | ||
${{ runner.os }}-gomod-conformance- |
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.
Conformance workflow uses a different cache as it also downloads and builds the conformance binary from bufbuild/protovalidate.
environment("GOBIN", bufCLIFile.parentFile.absolutePath) | ||
outputs.file(bufCLIFile) | ||
commandLine("go", "install", "github.com/bufbuild/buf/cmd/buf@latest") | ||
File(buf.asPath).setExecutable(true) |
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.
Instead of needing to download all the buf dependencies and build the CLI, we just download the Maven artifact and make it executable. This also has the benefit of including the artifact in the existing gradle CI cache.
} | ||
|
||
tasks.register<Exec>("installLicenseHeader") { | ||
description = "Installs the Buf license-header CLI." | ||
environment("GOBIN", bufLicenseHeaderCLIFile.parentFile.absolutePath) | ||
outputs.file(bufLicenseHeaderCLIFile) | ||
commandLine("go", "install", "github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@latest") | ||
commandLine("go", "install", "github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v${libs.versions.buf.get()}") |
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.
This should keep our builds stable as we no longer depend on latest.
maven { | ||
name = "buf" | ||
url = uri("https://buf.build/gen/maven") | ||
} |
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.
Originally this project used a generated SDK, but it now builds the conformance protos as part of this project so we can remove this.
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 really wanted to use a generated SDK for the newer conformance protos, but then we don't have the option to build for both lite and standard Java runtimes. Do we? If there's a way to pick which runtime to use in the URL, I'm all ears and will happily move this back to using generated SDKs (since that's what we use in conformance tests for other languages that generated SDKs support).
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.
Yep - you can use a lite
runtime artifact for plugins that support it: https://buf.build/docs/bsr/generated-sdks/maven#remote
testImplementation(libs.assertj) | ||
testImplementation(platform(libs.junit.bom)) | ||
testImplementation("org.junit.jupiter:junit-jupiter") | ||
testRuntimeOnly("org.junit.platform:junit-platform-launcher") |
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.
Fixes a gradle warning.
@@ -10,6 +11,7 @@ protobuf = "3.25.3" | |||
|
|||
[libraries] | |||
assertj = { module = "org.assertj:assertj-core", version.ref = "assertj" } | |||
buf = { module = "buf.build:buf", version.ref = "buf" } |
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.
🤞 that this will keep the Buf CLI updated with dependabot.
Update gradle to the latest version, build in CI with the latest LTS Java version, and remove unused Maven repository for generated SDKs (code generation has moved to happen in this project directly).