Publish #568
Publish #568
Conversation
|
Warning Rate limit exceeded@csikb has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds two new convention plugins (publishing and SBOM) implemented in buildSrc, introduces a java-platform build script, updates module includes and module dependencies, adjusts Java library jar naming, and applies the new conventions across client and server modules. Changes
Sequence DiagramsequenceDiagram
participant Gradle as Gradle Build
participant PluginReg as PublishingConventionPlugin
participant MavenPub as Maven Publish Plugin
participant Repo as GitHub Package Registry
Gradle->>PluginReg: apply(project)
activate PluginReg
PluginReg->>MavenPub: apply "maven-publish"
PluginReg->>MavenPub: configure PublishingExtension
PluginReg->>Repo: add repository "GitHubPackageRegistry" (URL)
Repo-->>PluginReg: accept credentials (GPR_USERNAME/GPR_PASSWORD)
PluginReg->>MavenPub: register MavenPublication (from components["java"])
PluginReg->>MavenPub: set groupId / set artifactId
alt root project group is blank
PluginReg->>Gradle: log warning about missing group
end
deactivate PluginReg
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
.github/workflows/gradle-publish.ymlis excluded by!**/*.yml.github/workflows/gradle.ymlis excluded by!**/*.yml
📒 Files selected for processing (8)
build.gradle.kts(1 hunks)buildSrc/build.gradle.kts(1 hunks)buildSrc/src/main/kotlin/hu/bsstudio/gradle/JavaLibraryConventionPlugin.kt(2 hunks)buildSrc/src/main/kotlin/hu/bsstudio/gradle/PublishingConventionPlugin.kt(1 hunks)client/build.gradle.kts(1 hunks)server/common/build.gradle.kts(1 hunks)server/model/build.gradle.kts(1 hunks)server/operation/build.gradle.kts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and publish
- GitHub Check: Integration test
🔇 Additional comments (6)
client/build.gradle.kts (1)
1-8: Publishing plugin usage onclientlooks consistentApplying
hu.bsstudio.gradle.publishing-conventionhere is consistent with other Java library modules and will publish the client artifact with the shared convention. No issues from a Gradle wiring standpoint.server/operation/build.gradle.kts (1)
1-10: Operation module publishing hook is wired correctlyAdding
hu.bsstudio.gradle.publishing-conventionhere matches the other Java library modules and should publishserver:operationwithout extra configuration.build.gradle.kts (1)
1-1: Confirm thathu.bsstudio.webis the intended groupId for all modulesThis root
groupwill drive thegroupIdof every published artifact via the publishing convention. Please double‑check it matches any existing coordinates/consumer expectations before publishing.server/model/build.gradle.kts (1)
1-16: Model module is correctly opted into publishingIncluding
hu.bsstudio.gradle.publishing-conventionhere is aligned with the other server modules and should publish the model JAR with the standardized conventions.server/common/build.gradle.kts (1)
1-8: Common module publishing is wired as expectedApplying
hu.bsstudio.gradle.publishing-conventionhere matches the pattern in other Java library modules; nothing problematic from a Gradle perspective.buildSrc/build.gradle.kts (1)
51-54: Publishing convention plugin registration looks correctThe plugin id and
implementationClassalign with the newPublishingConventionPluginimplementation and its usage in subprojects. No issues here.
| project.tasks.named<Jar>("jar") { | ||
| this.archiveBaseName.set("${project.rootProject.name}-${project.name}") | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid duplicating artifact name pattern between convention plugins
This archiveBaseName pattern is duplicated in PublishingConventionPlugin (for artifactId). To reduce drift risk, consider centralizing the "${project.rootProject.name}-${project.name}" convention in a shared helper or constant used by both plugins.
🤖 Prompt for AI Agents
In buildSrc/src/main/kotlin/hu/bsstudio/gradle/JavaLibraryConventionPlugin.kt
around lines 26 to 28, the archiveBaseName is set using the literal pattern
"${project.rootProject.name}-${project.name}" which is duplicated in
PublishingConventionPlugin; extract this pattern into a shared constant or
helper function (e.g., in a common util object in buildSrc) and replace the
literal in both plugins to reference that single constant/helper so both
jar.archiveBaseName and publishing.artifactId use the same centralized value.
| project.extensions.configure(PublishingExtension::class) { | ||
| repositories { | ||
| maven { | ||
| name = "GitHubPackageRegistry" | ||
| url = project.uri("https://maven.pkg.github.com/BSStudio/bss-web-admin-backend") | ||
| credentials { | ||
| username = project.providers.environmentVariable("GPR_USERNAME").orNull | ||
| password = project.providers.environmentVariable("GPR_PASSWORD").orNull | ||
| } | ||
| } | ||
| } | ||
| publications { | ||
| register<MavenPublication>(project.name) { | ||
| from(project.components.getByName("java")) | ||
| this.groupId = project.rootProject.group.toString() | ||
| this.artifactId = "${project.rootProject.name}-${project.name}" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make publishing configuration more robust to plugin order and reuse
The core wiring is sound, but a couple of improvements would make this safer and easier to maintain:
- The publishing logic assumes a
javacomponent is already present and the plugin is only ever applied to Java library projects. To decouple from plugin order and avoid accidental misuse, consider guarding this withplugins.withId("java-library") { ... }/withType<JavaPlugin>before accessingcomponents["java"]. - The
"${project.rootProject.name}-${project.name}"artifactId pattern duplicates thearchiveBaseNameconfiguration inJavaLibraryConventionPlugin. Centralizing this naming rule (shared helper/extension) would avoid divergence. - If
GPR_USERNAME/GPR_PASSWORDare unset, the failure will appear later as an auth error. You might want to validate these providers and log a clearer warning (or fail fast) when they are missing, to make local/CI misconfiguration easier to diagnose.
🤖 Prompt for AI Agents
In buildSrc/src/main/kotlin/hu/bsstudio/gradle/PublishingConventionPlugin.kt
around lines 18 to 36, the publishing block assumes a java component, duplicates
artifact naming, and silently continues when GPR credentials are missing; fix by
wrapping the publication setup in a plugins.withId("java-library") or
project.plugins.withType<JavaPlugin> guard so components["java"] is only
accessed when present, derive artifactId from the project's archiveBaseName (or
expose a shared helper/extension used by JavaLibraryConventionPlugin) instead of
constructing "${project.rootProject.name}-${project.name}" inline, and validate
the GPR_USERNAME/GPR_PASSWORD providers up-front - log a clear warning or fail
fast if they are null/empty to surface misconfiguration early.
buildSrc/src/main/kotlin/hu/bsstudio/gradle/PublishingConventionPlugin.kt
Outdated
Show resolved
Hide resolved
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ng convention plugin
…ng convention plugin
…ng convention plugin
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
buildSrc/src/main/kotlin/hu/bsstudio/gradle/PublishingConventionPlugin.kt (2)
16-34: Guard component access and validate credentials up front.The publishing configuration still has the robustness concerns flagged in the previous review:
Plugin order dependency: Accessing
components["java"]without a guard assumes thejava-libraryplugin is already applied. Wrap the publication setup inplugins.withId("java-library") { ... }to make this safe regardless of plugin application order.Duplicated artifactId pattern: The
"${project.rootProject.name}-${project.name}"pattern duplicates the naming logic inJavaLibraryConventionPlugin. Extract this into a shared helper to prevent divergence.Silent credential failure: If
GPR_USERNAMEorGPR_PASSWORDare unset, the build will fail later with an unclear auth error. Validate these providers during plugin application and fail fast with a clear message if missing.Apply this diff to add plugin guard and credential validation:
project.extensions.configure(PublishingExtension::class) { repositories { maven { name = "GitHubPackageRegistry" url = project.uri("https://maven.pkg.github.com/BSStudio/bss-web-admin-backend") credentials { + val gprUsername = project.providers.environmentVariable("GPR_USERNAME").orNull + val gprPassword = project.providers.environmentVariable("GPR_PASSWORD").orNull + + if (gprUsername.isNullOrBlank() || gprPassword.isNullOrBlank()) { + project.logger.warn("WARNING: GPR_USERNAME or GPR_PASSWORD not set. Publishing to GitHub Packages will fail.") + } - username = project.providers.environmentVariable("GPR_USERNAME").orNull - password = project.providers.environmentVariable("GPR_PASSWORD").orNull + username = gprUsername + password = gprPassword } } } - publications { - register<MavenPublication>(project.name) { - from(project.components.getByName("java")) - this.groupId = project.rootProject.group.toString() - this.artifactId = "${project.rootProject.name}-${project.name}" + + project.plugins.withId("java-library") { + publications { + register<MavenPublication>(project.name) { + from(project.components.getByName("java")) + this.groupId = project.rootProject.group.toString() + this.artifactId = "${project.rootProject.name}-${project.name}" + } } } }
36-41: Fail fast when root group is missing.Logging a warning allows the build to continue with an invalid
groupId, which will fail at publish time with a less clear error. As noted in the previous review, this should be a build failure during plugin application.Apply this diff to fail fast:
- if (project.rootProject.group - .toString() - .isBlank() - ) { - project.logger.warn("WARNING: The root project group is not set. Please set it to ensure proper publishing configuration.") - } + if (project.rootProject.group.toString().isBlank()) { + throw org.gradle.api.GradleException( + "Root project group is not set. Please set 'group' in the root build.gradle.kts to ensure proper publishing configuration." + ) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
.github/workflows/gradle-publish.ymlis excluded by!**/*.ymlgradle/libs.versions.tomlis excluded by!**/*.toml
📒 Files selected for processing (8)
build.gradle.kts(1 hunks)buildSrc/build.gradle.kts(3 hunks)buildSrc/src/main/kotlin/hu/bsstudio/gradle/PublishingConventionPlugin.kt(1 hunks)buildSrc/src/main/kotlin/hu/bsstudio/gradle/SbomConventionPlugin.kt(1 hunks)client/build.gradle.kts(1 hunks)server/common/build.gradle.kts(1 hunks)server/model/build.gradle.kts(1 hunks)server/operation/build.gradle.kts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T16:19:58.743Z
Learnt from: csikb
Repo: BSStudio/bss-web-admin-backend PR: 305
File: buildSrc/src/main/kotlin/spotless-conventions.gradle.kts:11-14
Timestamp: 2024-10-08T16:19:58.743Z
Learning: The `cleanthat()` configuration option is a valid Spotless plugin for Gradle configuration, as documented in the Spotless plugin's GitHub README.
Applied to files:
buildSrc/build.gradle.kts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build and publish
- GitHub Check: Gradle
- GitHub Check: Build
- GitHub Check: Integration test
🔇 Additional comments (10)
server/operation/build.gradle.kts (1)
9-10: LGTM! Convention plugins applied correctly.The addition of
publishing-conventionandsbom-conventionenables Maven publishing and SBOM generation for this module. The plugin order (java-library-convention applied first) is appropriate.server/common/build.gradle.kts (1)
7-8: LGTM! Convention plugins applied correctly.The publishing and SBOM conventions are properly applied to enable artifact publishing for this module.
server/model/build.gradle.kts (1)
9-12: LGTM! Convention plugins applied correctly.The publishing and SBOM conventions are properly added to enable artifact publishing for the model module.
client/build.gradle.kts (1)
7-8: LGTM! Convention plugins applied correctly.The client module now has publishing and SBOM generation enabled through the convention plugins.
buildSrc/src/main/kotlin/hu/bsstudio/gradle/SbomConventionPlugin.kt (1)
8-12: LGTM! Simple delegation pattern is appropriate.The plugin cleanly delegates to
CyclonedxPluginto provide SBOM generation. If you need to customize CycloneDX behavior (output format, component analysis depth, etc.) in the future, you can add configuration here.buildSrc/build.gradle.kts (2)
1-1: Verify the need for buildSrc version.Setting a version on the
buildSrcmodule is unusual since it's not typically published as a separate artifact. If this is for internal tracking or documentation, it's fine. Otherwise, this line may be unnecessary.
54-61: LGTM! Plugin registration is correct.The two new convention plugins are properly registered with matching IDs and implementation classes. The plugin IDs follow the existing naming convention and are used correctly in the module build scripts.
build.gradle.kts (3)
1-1: LGTM! Root group properly configured.Setting the root project group is essential for the publishing conventions to work correctly. The group
"hu.bsstudio.web"will be inherited by all subprojects.
3-7: LGTM! Plugin configuration appropriate for platform module.The root project correctly uses
java-platformfor BOM functionality and appliesmaven-publishdirectly (rather thanpublishing-convention) since it publishes from thejavaPlatformcomponent instead of thejavacomponent.
9-16: LGTM! Platform constraints correctly defined.The dependency constraints enable consumers to import the platform and get aligned versions of all library modules. This is the correct pattern for a Gradle platform module.
| publishing { | ||
| publications { | ||
| create<MavenPublication>("bssWebAdminPlatform") { | ||
| from(components["javaPlatform"]) | ||
| this.groupId = project.rootProject.group.toString() | ||
| this.artifactId = "${project.rootProject.name}-${project.name}" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using dynamic publication name and centralizing artifactId pattern.
Two observations:
-
Hardcoded publication name: The publication is named
"bssWebAdminPlatform"but could useproject.namefor consistency. If the root project is renamed, this would need manual updating. -
ArtifactId pattern duplication: The
"${project.rootProject.name}-${project.name}"pattern now appears in three places:JavaLibraryConventionPlugin,PublishingConventionPlugin, and here. Consider extracting this into a shared helper or extension property to maintain consistency.
Apply this diff for the publication name:
publishing {
publications {
- create<MavenPublication>("bssWebAdminPlatform") {
+ create<MavenPublication>(project.name) {
from(components["javaPlatform"])
this.groupId = project.rootProject.group.toString()
this.artifactId = "${project.rootProject.name}-${project.name}"
}
}
}For the artifactId pattern, consider creating a shared extension in buildSrc that all three locations can use.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| publishing { | |
| publications { | |
| create<MavenPublication>("bssWebAdminPlatform") { | |
| from(components["javaPlatform"]) | |
| this.groupId = project.rootProject.group.toString() | |
| this.artifactId = "${project.rootProject.name}-${project.name}" | |
| } | |
| } | |
| } | |
| publishing { | |
| publications { | |
| create<MavenPublication>(project.name) { | |
| from(components["javaPlatform"]) | |
| this.groupId = project.rootProject.group.toString() | |
| this.artifactId = "${project.rootProject.name}-${project.name}" | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In build.gradle.kts around lines 18–26, the publication uses a hardcoded name
("bssWebAdminPlatform") and duplicates the artifactId pattern
("${project.rootProject.name}-${project.name}") used in multiple places; change
the publication name to use project.name (e.g.,
create<MavenPublication>(project.name)) and replace the inline artifactId
expression with a shared property; add a small extension/helper in buildSrc
(e.g., project.extensions.create or a Kotlin object) that exposes the artifactId
pattern (computed from rootProject.name and project.name) and update
JavaLibraryConventionPlugin, PublishingConventionPlugin and this
build.gradle.kts to read the artifactId from that centralized helper so the
pattern is maintained in one place.
…ng convention plugin
…ng convention plugin
|
Summary by CodeRabbit
Chores
New Features
✏️ Tip: You can customize this high-level summary in your review settings.