[Java] Build rocketmq-proto from submodule instead of depending on Maven Central#1287
Conversation
e855bfa to
a3aca0c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1287 +/- ##
============================================
+ Coverage 53.26% 53.67% +0.40%
- Complexity 651 785 +134
============================================
Files 208 221 +13
Lines 14303 14466 +163
Branches 5845 5577 -268
============================================
+ Hits 7619 7764 +145
+ Misses 6308 6303 -5
- Partials 376 399 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot
Summary
This PR adds a proto module to the Java Maven reactor that compiles .proto files directly from the protos/ git submodule, replacing the dependency on a pre-published rocketmq-proto artifact from Maven Central. This decouples Java client development from proto release cycles and aligns Java with other language clients (C++, Rust, C#) that already compile proto from the submodule.
Findings
-
[Info]
java/proto/pom.xml:63—protobuf-maven-pluginversion0.6.1is used. This is the latest release of the plugin and works well, but note that the plugin has been relatively inactive. If you ever need newer protoc features, consider evaluatingio.github.ascopes:protobuf-maven-pluginas a modern alternative. Not blocking. -
[Info]
java/pom.xml:52— Therocketmq-protoversion is now tied to${project.version}(5.2.1), whereas previously it was an independent2.1.2. This is correct for reactor builds, but if anyone tries to consumerocketmq-protoas a standalone artifact published separately in the future, the version coupling may cause confusion. Consider adding a brief comment in the POM explaining this design choice. -
[Info]
.github/workflows/java_build.ymland.github/workflows/java_coverage.yml— CI checkout steps correctly addsubmodules: recursive. I verified that all checkout steps across both workflows are updated (2 injava_build.yml, 1 injava_coverage.yml). Good. -
[Info]
java/proto/pom.xml:62—protoSourceRootuses${project.basedir}/../../protos, which correctly resolves fromjava/proto/→ repo root →protos/. The path is correct. -
[Info]
java/proto/pom.xml— Checkstyle, SpotBugs, and JaCoCo are all correctly skipped for the generated proto code. This is appropriate since generated code should not be linted or counted for coverage.
Suggestions
-
Minor: Consider pinning the
protossubmodule commit in a comment or README so contributors know whichrocketmq-apisSHA is expected. The submodule pointer update (68c2cc9→bc8e184) is clear in the diff, but a note injava/proto/pom.xmlor a top-level README would help future contributors understand the relationship. -
Minor: The
java_build.ymlusesactions/checkout@v3whilejava_coverage.ymlusesactions/checkout@master. Consider standardizing on a pinned major version (e.g.,@v4) for reproducibility. This is pre-existing and not introduced by this PR, just a general observation.
Overall
The change is clean, well-structured, and solves a real CI breakage (PR #1237 bumped proto to 2.2.0 which isn't on Maven Central yet). The approach matches what other language clients already do. LGTM.
Automated review by github-manager-bot
a3aca0c to
77f3f28
Compare
77f3f28 to
fa5742d
Compare
Summary
Add a
protomodule to the Java Maven reactor that compiles.protofiles from theprotos/submodule (rocketmq-apis) into arocketmq-protoJAR, replacing the dependency on a pre-published Maven Central artifact.Closes #1286
Changes
protos/submodule to latestmain(bc8e184), which includes theclient_propertiesfield from rocketmq-apis#108.java/proto/pom.xmlusingprotobuf-maven-pluginto compile the 3.protofiles (definition.proto,service.proto,admin.proto) and generate gRPC stubs.protoas the first module in the parent POM reactor build order, so downstream modules (client-apis,client,client-shade) resolverocketmq-protofrom the reactor instead of Maven Central.java_build.yml,java_coverage.yml) to checkout submodules withsubmodules: recursive.Why
rocketmq-prototo2.2.0, but that version is not yet published to Maven Central, causing all Java CI jobs to fail.Build Order