Initial project setup#5
Merged
Merged
Conversation
- Initialize Gradle 9.4.1 with java-library plugin and BitWeb conventions - Add MontonioSdkConfiguration with sandbox base URL default - Add GitHub Actions CI workflow for PRs (Java 17, 21, 25) - Add .sdkmanrc for Java 17, .gitignore, CLAUDE.md Closes #1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughInitial project scaffolding: Gradle build and wrapper, CI workflow, VCS configs, publication/signing scripts, test and coverage tasks, a simple SDK configuration class with default sandbox URL, and corresponding unit test. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
build.gradle (1)
1-5: Redundant buildscript classpath declaration.The
org.owasp:dependency-check-gradlein the buildscript classpath (line 3) is redundant since the plugin is already applied with a version in thepluginsblock (line 9). The buildscript block can be removed.♻️ Suggested removal
-buildscript { - dependencies { - classpath 'org.owasp:dependency-check-gradle' - } -} - plugins { id 'java'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle` around lines 1 - 5, Remove the redundant buildscript classpath declaration: delete the entire buildscript { dependencies { classpath 'org.owasp:dependency-check-gradle' } } block because the dependency-check Gradle plugin is already applied via the plugins block (org.owasp:dependency-check-gradle with a version); keep the plugins block intact and ensure no other code depends on the removed buildscript symbol.library.gradle (1)
62-64: Reconsider including source files in the main JAR.Including
sourceSets.main.allSourcein the main JAR bundles source code alongside compiled classes, which is unusual. Sources are typically only in the-sources.jar(already enabled viawithSourcesJar()). This doubles the JAR size and may expose implementation details.♻️ Consider removing this
-jar { - from sourceSets.main.allSource -}If you intentionally want sources in the main JAR (e.g., for IDE debugging without attaching sources), please add a comment explaining the rationale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@library.gradle` around lines 62 - 64, The jar task currently includes sourceSets.main.allSource which packages source files into the main JAR (duplicating withSourcesJar()); remove sourceSets.main.allSource from the jar configuration unless you intentionally need sources in the runtime artifact, or if keeping it add a clear comment explaining the rationale (e.g., "include sources in main JAR for IDE debugging without attaching sources"). Update the jar task (referencing jar and sourceSets.main.allSource) so only compiled classes are packaged and rely on withSourcesJar() to publish sources, or document why this special behavior is required.test.gradle (1)
21-27: JaCoCo report tasks should explicitly depend on their test tasks.The
jacocoUnitTestReportandjacocoIntegrationTestReporttasks referenceexecutionDatafrom test tasks but don't declaredependsOn. This means running the report task directly may fail if tests haven't been executed.♻️ Proposed fix
tasks.register('jacocoUnitTestReport', JacocoReport) { + dependsOn unitTest sourceSets sourceSets.main executionData unitTest reports { xml.required = true } } tasks.register('jacocoIntegrationTestReport', JacocoReport) { + dependsOn integrationTest sourceSets sourceSets.main executionData integrationTest reports { xml.required = true } }Also applies to: 29-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test.gradle` around lines 21 - 27, The jacoco report tasks (jacocoUnitTestReport and jacocoIntegrationTestReport) declare executionData from test tasks but don’t declare task dependencies, so running them directly can fail; update the task registrations for jacocoUnitTestReport and jacocoIntegrationTestReport to add explicit dependsOn on the corresponding test tasks (unitTest and integrationTest) — e.g., ensure jacocoUnitTestReport.dependsOn(unitTest) and jacocoIntegrationTestReport.dependsOn(integrationTest) or use dependsOn(tasks.named('unitTest')) / dependsOn(tasks.named('integrationTest')) so the tests run before the reports are generated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle`:
- Around line 9-11: The junit BOM version is pinned to 5.12.2 in build.gradle;
update the org.junit:junit-bom dependency declaration (look for the string
"org.junit:junit-bom" and the version "5.12.2") to the latest 5.14.3 so the
project uses the current JUnit BOM with bug and security fixes.
In `@gradle.properties`:
- Line 1: Remove the insecure Gradle property that disables checksum
verification: delete the line
`systemProp.org.gradle.internal.publish.checksums.insecure=true` from
gradle.properties so Maven publish integrity checks remain enforced; if there
was a specific reason for disabling checksums, replace it with documented,
secure alternatives or add a comment explaining and tracking the justification
instead of leaving the insecure flag in place.
In `@gradlew.bat`:
- Around line 1-94: gradlew.bat currently uses LF-only line endings which breaks
Windows batch parsing; convert gradlew.bat to CRLF and add a .gitattributes
entry to force CRLF for the batch wrapper (e.g., set "gradlew.bat text eol=crlf"
and ensure gradlew stays as appropriate), then re-normalize in Git (git add
--renormalize gradlew.bat and commit) so labels like :execute and
:findJavaFromJavaHome parse correctly on Windows.
In `@library.gradle`:
- Line 38: Replace the bitwise AND operator (&) with the logical AND operator
(&&) in the conditional checks that test
project.hasProperty('mavenCentralUsername') &
project.hasProperty('mavenCentralPassword') (and the analogous occurrence later
in the file), so the if expressions use short-circuit evaluation (e.g., change
the expression calling project.hasProperty(...) & project.hasProperty(...) to
use &&).
- Around line 49-54: The signing block currently always calls sign
publishing.publications.mavenJava even when keys are absent; update the signing
configuration (the signing block) to only call sign
publishing.publications.mavenJava when signing is required and keys exist: wrap
or gate the sign(...) call behind the same condition that checks
project.hasProperty("signingKey") && project.hasProperty("signingPassword") or
use signing.required { ... } logic to skip signing for snapshots/local builds;
ensure useInMemoryPgpKeys(project.signingKey, project.signingPassword) remains
inside that conditional so signing only runs when keys are configured.
- Around line 26-30: The SCM block currently uses incorrect prefixes; update the
scm configuration in the Gradle publishing setup (the scm { ... } block) so that
both connection and developerConnection use the required "scm:git:" prefix
(e.g., change "scm:https://..." to "scm:git:https://..." and "scm:git@..." to
"scm:git:git@..."), leaving the url field unchanged to satisfy Maven Central's
SCM URL format requirements.
In `@test.gradle`:
- Around line 37-40: The task declaration for testAndReport currently registers
the task as type Test which creates an empty test execution; change the
registration to use DefaultTask instead of Test so it only orchestrates other
tasks (update the tasks.register('testAndReport', Test) call to use DefaultTask
and keep the existing mustRunAfter and dependsOn configuration for 'unitTest',
'integrationTest', 'jacocoUnitTestReport', and 'jacocoIntegrationTestReport').
---
Nitpick comments:
In `@build.gradle`:
- Around line 1-5: Remove the redundant buildscript classpath declaration:
delete the entire buildscript { dependencies { classpath
'org.owasp:dependency-check-gradle' } } block because the dependency-check
Gradle plugin is already applied via the plugins block
(org.owasp:dependency-check-gradle with a version); keep the plugins block
intact and ensure no other code depends on the removed buildscript symbol.
In `@library.gradle`:
- Around line 62-64: The jar task currently includes sourceSets.main.allSource
which packages source files into the main JAR (duplicating withSourcesJar());
remove sourceSets.main.allSource from the jar configuration unless you
intentionally need sources in the runtime artifact, or if keeping it add a clear
comment explaining the rationale (e.g., "include sources in main JAR for IDE
debugging without attaching sources"). Update the jar task (referencing jar and
sourceSets.main.allSource) so only compiled classes are packaged and rely on
withSourcesJar() to publish sources, or document why this special behavior is
required.
In `@test.gradle`:
- Around line 21-27: The jacoco report tasks (jacocoUnitTestReport and
jacocoIntegrationTestReport) declare executionData from test tasks but don’t
declare task dependencies, so running them directly can fail; update the task
registrations for jacocoUnitTestReport and jacocoIntegrationTestReport to add
explicit dependsOn on the corresponding test tasks (unitTest and
integrationTest) — e.g., ensure jacocoUnitTestReport.dependsOn(unitTest) and
jacocoIntegrationTestReport.dependsOn(integrationTest) or use
dependsOn(tasks.named('unitTest')) / dependsOn(tasks.named('integrationTest'))
so the tests run before the reports are generated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08d9d713-b6d8-4538-927b-7b3445626f17
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (14)
.github/workflows/build.yml.gitignore.sdkmanrcCLAUDE.mdbuild.gradlegradle.propertiesgradle/wrapper/gradle-wrapper.propertiesgradlewgradlew.batlibrary.gradlesettings.gradlesrc/main/java/ee/bitweb/montonio/sdk/MontonioSdkConfiguration.javasrc/test/java/ee/bitweb/montonio/sdk/MontonioSdkConfigurationTest.javatest.gradle
- Update junit-bom 5.12.2 → 5.14.3 - Remove insecure checksum property from gradle.properties - Fix SCM connection URL formats for Maven Central - Use logical AND (&&) instead of bitwise AND (&) in library.gradle - Make signing conditional on key availability - Change testAndReport to DefaultTask (not Test) - Add .gitattributes for gradlew.bat CRLF line endings Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test.gradle (1)
37-41: Use lazy task references instead of eager lookup intestAndReport.Line 40 uses
tasks.getByName('integrationTest'), which eagerly realises the task and bypasses Gradle's configuration avoidance. Usetasks.named()to keep configuration lazy.♻️ Proposed refactor
tasks.register('testAndReport') { group = "verification" description = "Runs unit and integration tests with coverage reports" - tasks.getByName('integrationTest').mustRunAfter('unitTest') + tasks.named('integrationTest') { + mustRunAfter(tasks.named('unitTest')) + } dependsOn('unitTest', 'integrationTest', 'jacocoUnitTestReport', 'jacocoIntegrationTestReport') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test.gradle` around lines 37 - 41, The task registration for testAndReport currently uses an eager lookup tasks.getByName('integrationTest') which realizes the task; change that to use lazy references via tasks.named('integrationTest') for mustRunAfter, and convert the dependsOn list to use lazy/task provider references (e.g., tasks.named('unitTest'), tasks.named('integrationTest'), and the jacoco report tasks via tasks.named(...)) so configuration avoidance is preserved; update references in the testAndReport closure for mustRunAfter and dependsOn to use tasks.named(...) instead of tasks.getByName(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@library.gradle`:
- Around line 16-31: The POM's pom { ... } metadata is missing the required
developers section causing Maven Central validation to fail; update the pom
block to include a developers { developer { id, name, email, url } } entry (add
one or more developer entries) with real maintainer values so the publication
has the mandatory developers metadata; ensure the developers block is placed
alongside name/description/url/licenses/scm inside the existing pom closure
(reference: the pom block in library.gradle).
- Around line 62-64: The build currently forces sources into the main runtime
artifact via the jar { from sourceSets.main.allSource } block; remove that jar
block so the main JAR stays lean and rely on the existing withSourcesJar()
configuration to produce the separate sources JAR. Locate and delete the jar {
from sourceSets.main.allSource } block (reference: jar and
sourceSets.main.allSource) and keep withSourcesJar() as-is.
---
Nitpick comments:
In `@test.gradle`:
- Around line 37-41: The task registration for testAndReport currently uses an
eager lookup tasks.getByName('integrationTest') which realizes the task; change
that to use lazy references via tasks.named('integrationTest') for mustRunAfter,
and convert the dependsOn list to use lazy/task provider references (e.g.,
tasks.named('unitTest'), tasks.named('integrationTest'), and the jacoco report
tasks via tasks.named(...)) so configuration avoidance is preserved; update
references in the testAndReport closure for mustRunAfter and dependsOn to use
tasks.named(...) instead of tasks.getByName(...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5cc49dea-7a1d-4a52-a11b-25570bc3a398
📒 Files selected for processing (5)
.gitattributesbuild.gradlegradle.propertieslibrary.gradletest.gradle
✅ Files skipped from review due to trivial changes (3)
- gradle.properties
- .gitattributes
- build.gradle
Address remaining CodeRabbit review comments: add required developers block for Maven Central publication and remove jar block that duplicated sources into the main artifact. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
build.gradle,test.gradle,library.gradle)MontonioSdkConfigurationwith sandbox base URL default and test.sdkmanrc,.gitignore,CLAUDE.mdCloses #1
Test plan
./gradlew buildpasses locally./gradlew testpasses (verifies sandbox base URL default)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Tests
Documentation