-
Notifications
You must be signed in to change notification settings - Fork 7
[fix] Remove the pulsar-bom import #22
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
Conversation
3e75ce7 to
d6f801a
Compare
…n version, and a unified pulsar version is not strictly necessary. Each project can import it independently. If using the default version, use default.pulsar.version.
d6f801a to
695678d
Compare
| <type>pom</type> | ||
| <scope>import</scope> | ||
| </dependency> | ||
| <dependency> |
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.
Why remove this BOM? Did you encounter any problems?
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.
Pull Request Overview
This PR refactors Maven dependency management by renaming the pulsar.version property to default.pulsar.version and removing the centralized pulsar-bom dependency management. The changes allow for more explicit version control of Apache Pulsar dependencies across child modules.
- Renamed property
pulsar.versiontodefault.pulsar.versionin parent POM - Removed
pulsar-bomfrom centralized dependency management - Added explicit version declarations for Pulsar dependencies in child modules
- Removed duplicate
pulsar-client-admindependency inpulsar-admin-mcp-contrib
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Renamed pulsar.version property to default.pulsar.version and removed pulsar-bom from dependency management |
| pulsar-transaction-contrib/pom.xml | Added explicit version reference using ${default.pulsar.version} for pulsar-client-all |
| pulsar-rpc-contrib/src/main/resources/pulsar-container.properties | Updated property reference from ${pulsar.version} to ${default.pulsar.version} |
| pulsar-rpc-contrib/pom.xml | Added explicit version references for pulsar-client and pulsar-client-admin dependencies |
| pulsar-client-common-contrib/pom.xml | Added explicit version reference for pulsar-client-all dependency |
| pulsar-admin-mcp-contrib/pom.xml | Added explicit version references and removed duplicate pulsar-client-admin dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <dependency> | ||
| <groupId>org.apache.pulsar</groupId> | ||
| <artifactId>pulsar-client-admin</artifactId> | ||
| <version>${default.pulsar.version}</version> |
Copilot
AI
Nov 4, 2025
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.
The pulsar-client-admin dependency is missing the <scope>test</scope> that was present in the removed duplicate declaration. If this dependency is only needed for tests, the test scope should be retained on line 42.
| <version>${default.pulsar.version}</version> | |
| <version>${default.pulsar.version}</version> | |
| <scope>test</scope> |
Remove the pulsar-bom import; each module is free to use its own version, and a unified pulsar version is not strictly necessary. Each project can import it independently. If using the default version, use default.pulsar.version.