MOSIP-37901 - Exclude MOSIP services from being published or released…#302
MOSIP-37901 - Exclude MOSIP services from being published or released…#302NidhiKumari0201 wants to merge 1 commit into
Conversation
… to nexus Signed-off-by: Nidhi0201 <nidhi.k@cyberpwn.com>
WalkthroughThis pull request adds new execution configurations for the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
commons-packet/pom.xml (1)
39-53:⚠️ Potential issue | 🟠 Major
commons-packet-manageractively deploys viamaven-deploy-pluginandcentral-publishing-maven-plugin— both need disabling.The aggregator POM lacks
maven-deploy-pluginskip configuration, and the child modulecommons-packet-manageris actively configured to deploy:
Aggregator (
commons-packet/pom.xml) needsmaven-deploy-pluginskip. Runningmvn deploywill attempt to push this aggregator artifact tohttps://central.sonatype.com/api/v1/publishervia thedistributionManagementblock (lines 44–53).
commons-packet-managerproduces a deployable artifact without publish restrictions. Itsmaven-deploy-pluginis set to<phase>deploy</phase>, meaningmvn deploywill execute. Thecentral-publishing-maven-pluginis only set to<autoPublish>false</autoPublish>, which prevents auto-publishing but does not block the deploy goal entirely. To match the stated objective, add<skip>true</skip>and overridedefault-deploytophase=noneincommons-packet-manager/pom.xmlas well.Remove
<distributionManagement>blocks from both POMs since they point to Sonatype Central and contradict the goal of excluding these MOSIP services from publication. Alternatively, move the skip configuration into parentpluginManagementso all modules inherit it.Suggested addition for aggregator POM
<plugins> <plugin> <groupId>org.springframework.boot</groupId> <artifactId>spring-boot-maven-plugin</artifactId> <version>3.2.3</version> </plugin> + <plugin> + <artifactId>maven-deploy-plugin</artifactId> + <version>${maven.deploy.plugin.version}</version> + <configuration> + <skip>true</skip> + </configuration> + <executions> + <execution> + <id>default-deploy</id> + <phase>none</phase> + <goals> + <goal>deploy</goal> + </goals> + </execution> + </executions> + </plugin>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commons-packet/pom.xml` around lines 39 - 53, Add configuration to prevent accidental publishing: in the aggregator POM (commons-packet/pom.xml) disable the maven-deploy-plugin by setting its skip to true (or override the default-deploy execution to phase=none) so the aggregator won’t run deploy on mvn deploy, and remove or relocate the distributionManagement block that points to Sonatype; in the commons-packet-manager child POM disable its maven-deploy-plugin by adding <skip>true</skip> and override the default-deploy execution to phase=none (or inherit the skip via parent pluginManagement), and ensure central-publishing-maven-plugin remains with autoPublish false (or is removed) so no module runs the deploy goal or publishes to the Sonatype repository.
♻️ Duplicate comments (1)
commons-packet/pom.xml (1)
155-163: 🧹 Nitpick | 🔵 TrivialSame caveat as in the service POM: verify the auto-injected execution id and consider
skipPublishing.Identical change as in
commons-packet/commons-packet-service/pom.xml— the override depends on the plugin's lifecycle-injected execution being nameddefault-publish, which is Maven convention but not guaranteed for an extension-injected goal. Prefer the plugin's documented<skipPublishing>true</skipPublishing>configuration (or drop<extensions>true</extensions>) to make intent explicit and not rely on internal id naming.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commons-packet/pom.xml` around lines 155 - 163, The POM currently disables the auto-injected publish execution by referencing an assumed execution id "default-publish" and setting its phase to "none"; instead, update the plugin configuration for the publish goal used in this module (the execution with id "default-publish" / goal "publish") to explicitly use the plugin-supported flag (e.g., add <skipPublishing>true</skipPublishing>) or remove/avoid <extensions>true</extensions> so you don't rely on an implementation-specific execution id, ensuring the publish behavior is disabled via the plugin's documented configuration rather than by overriding a possibly non‑existent execution id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commons-packet/commons-packet-service/pom.xml`:
- Around line 184-192: The current execution override for the
central-publishing-maven-plugin uses an execution id of default-publish with
<phase>none</phase> which may not match the plugin's auto-injected execution id;
verify the actual injected id by running mvn -X help:describe
-Dplugin=org.sonatype.central:central-publishing-maven-plugin -Ddetail=true (or
mvn -X deploy -DskipTests) and if the id differs, either remove this execution
override and instead set the plugin config property
<skipPublishing>true</skipPublishing> inside the central-publishing-maven-plugin
configuration (or drop <extensions>true</extensions> entirely to avoid
auto-binding), and make the same change in the duplicated block in
commons-packet/pom.xml so both POMs stay in sync.
---
Outside diff comments:
In `@commons-packet/pom.xml`:
- Around line 39-53: Add configuration to prevent accidental publishing: in the
aggregator POM (commons-packet/pom.xml) disable the maven-deploy-plugin by
setting its skip to true (or override the default-deploy execution to
phase=none) so the aggregator won’t run deploy on mvn deploy, and remove or
relocate the distributionManagement block that points to Sonatype; in the
commons-packet-manager child POM disable its maven-deploy-plugin by adding
<skip>true</skip> and override the default-deploy execution to phase=none (or
inherit the skip via parent pluginManagement), and ensure
central-publishing-maven-plugin remains with autoPublish false (or is removed)
so no module runs the deploy goal or publishes to the Sonatype repository.
---
Duplicate comments:
In `@commons-packet/pom.xml`:
- Around line 155-163: The POM currently disables the auto-injected publish
execution by referencing an assumed execution id "default-publish" and setting
its phase to "none"; instead, update the plugin configuration for the publish
goal used in this module (the execution with id "default-publish" / goal
"publish") to explicitly use the plugin-supported flag (e.g., add
<skipPublishing>true</skipPublishing>) or remove/avoid
<extensions>true</extensions> so you don't rely on an implementation-specific
execution id, ensuring the publish behavior is disabled via the plugin's
documented configuration rather than by overriding a possibly non‑existent
execution id.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdc5f7ed-6712-41ee-aa6d-da6b379ec4c3
📒 Files selected for processing (2)
commons-packet/commons-packet-service/pom.xmlcommons-packet/pom.xml
| <executions> | ||
| <execution> | ||
| <id>default-publish</id> | ||
| <phase>none</phase> | ||
| <goals> | ||
| <goal>publish</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Verify default-publish is the actual auto-injected execution id, and consider the documented skipPublishing alternative.
central-publishing-maven-plugin uses <extensions>true</extensions> and registers as a Maven lifecycle participant that dynamically injects its publish goal into the deploy phase. Overriding it with an execution id of default-publish and <phase>none</phase> relies on Maven's default-<goalId> naming convention for that injection. With this plugin that convention is not officially documented for the injected execution — if the plugin uses any other id, this override will silently do nothing and the goal will still run on mvn deploy.
A cleaner, documented alternative is the plugin's own configuration switch <skipPublishing>true</skipPublishing> (intended for "creating only the bundle but skipping uploading and publishing"). Even simpler, since the PR's stated goal is to exclude these services from Nexus/Central entirely, you could drop <extensions>true</extensions> (or remove the plugin block) so the plugin no longer auto-binds to the lifecycle at all.
This block is duplicated verbatim in commons-packet/pom.xml — please keep them in sync (or define it once in the aggregator and inherit, which is currently not possible since this POM does not declare a <parent>).
♻️ Documented alternative using skipPublishing
<plugin>
<groupId>org.sonatype.central</groupId>
<artifactId>central-publishing-maven-plugin</artifactId>
<version>${central.publishing.maven.plugin.version}</version>
<extensions>true</extensions>
- <executions>
- <execution>
- <id>default-publish</id>
- <phase>none</phase>
- <goals>
- <goal>publish</goal>
- </goals>
- </execution>
- </executions>
<configuration>
<publishingServerId>ossrh</publishingServerId>
<autoPublish>false</autoPublish>
+ <skipPublishing>true</skipPublishing>
</configuration>
</plugin>Please run a quick mvn -X deploy -DskipTests (or just mvn -X help:describe -Dplugin=org.sonatype.central:central-publishing-maven-plugin -Ddetail=true) and confirm that the execution id Maven prints for the auto-injected publish goal is indeed default-publish — that is the linchpin assumption of this change.
📝 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.
| <executions> | |
| <execution> | |
| <id>default-publish</id> | |
| <phase>none</phase> | |
| <goals> | |
| <goal>publish</goal> | |
| </goals> | |
| </execution> | |
| </executions> | |
| <plugin> | |
| <groupId>org.sonatype.central</groupId> | |
| <artifactId>central-publishing-maven-plugin</artifactId> | |
| <version>${central.publishing.maven.plugin.version}</version> | |
| <extensions>true</extensions> | |
| <configuration> | |
| <publishingServerId>ossrh</publishingServerId> | |
| <autoPublish>false</autoPublish> | |
| <skipPublishing>true</skipPublishing> | |
| </configuration> | |
| </plugin> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@commons-packet/commons-packet-service/pom.xml` around lines 184 - 192, The
current execution override for the central-publishing-maven-plugin uses an
execution id of default-publish with <phase>none</phase> which may not match the
plugin's auto-injected execution id; verify the actual injected id by running
mvn -X help:describe
-Dplugin=org.sonatype.central:central-publishing-maven-plugin -Ddetail=true (or
mvn -X deploy -DskipTests) and if the id differs, either remove this execution
override and instead set the plugin config property
<skipPublishing>true</skipPublishing> inside the central-publishing-maven-plugin
configuration (or drop <extensions>true</extensions> entirely to avoid
auto-binding), and make the same change in the duplicated block in
commons-packet/pom.xml so both POMs stay in sync.
1 participant
… to nexus
Summary by CodeRabbit