#1830 https://github.com/mosip/commons/issues/1830 Move release-1.3.x changes to develop#128
Conversation
WalkthroughThe pull request bumps the kernel-bio-converter Maven version to 1.4.0-SNAPSHOT and associated dependency properties, sets Helm CHART_VERSION to 1.3.0, reorganizes Dockerfile environment setup, adds AGENTS.md documentation, adjusts README/NOTICE whitespace, and expands IConverterApi Javadoc error codes. ChangesRelease 1.4.0 Version Bump and Configuration
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@kernel-bio-converter/pom.xml`:
- Line 8: The pom.xml currently sets the module <version> to 1.3.0 (a release)
but run_local.bat and the branch convention expect a SNAPSHOT; fix the mismatch
by making them consistent: either change the pom.xml <version> to 1.3.0-SNAPSHOT
(preferred for develop) or update run_local.bat to reference
kernel-bio-converter-1.3.0.jar; ensure the <version> tag in pom.xml and the jar
name referenced in run_local.bat match exactly.
- Around line 77-79: Update the Sonar exclusion entries to target this module's
Spring Boot main class: replace references to AuditManagerBootApplication.java
with KernelBioConverterApplication.java in the pom's sonar.exclusions and
sonar.coverage.exclusions. Edit the pom.xml properties where sonar.exclusions
and sonar.coverage.exclusions are declared so they include
**/KernelBioConverterApplication.java (instead of
**/AuditManagerBootApplication.java) ensuring both exclusion lists match the
module's actual boot class name.
In `@kernel-bio-converter/run_local.bat`:
- Line 1: The batch script run_local.bat currently has Unix LF-only line endings
which can break the Windows batch parser; convert run_local.bat to use CRLF line
endings (e.g., via your editor's "CRLF" setting, dos2unix/win32 line-ending
tools reversed, or git's core.autocrlf/.gitattributes) and re-save/commit the
file so the Java invocation line (the java -Dloader.path=";" ... -jar
target/kernel-bio-converter-1.3.0-SNAPSHOT.jar string) uses CRLF line endings on
Windows.
- Line 1: Update the jar filename in the run_local.bat invocation so it matches
the version declared in pom.xml (change
target/kernel-bio-converter-1.3.0-SNAPSHOT.jar to
target/kernel-bio-converter-1.3.0.jar), or replace the hardcoded name with a
more robust pattern/variable (e.g., use the Maven project version or a wildcard
like target/kernel-bio-converter-*.jar) so the java -jar call in run_local.bat
always points to the built artifact.
In `@README.md`:
- Around line 2-3: The README badge URLs currently reference branch
"release-1.3.x"; update both badge links (the GitHub Actions badge URL and the
SonarCloud badge URL parameters) to use "develop" so the images and links
reflect the develop branch after merge—specifically edit the two badge markdown
lines that include "branch=release-1.3.x" and "&branch=release-1.3.x" to use
"develop".
🪄 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: 2272c995-37d2-437c-82c4-a70d4f91e6fc
📒 Files selected for processing (9)
NOTICEREADME.mddeploy/install.shhelm/converters/Chart.yamlhelm/converters/README.mdhelm/converters/values.yamlkernel-bio-converter/Dockerfilekernel-bio-converter/pom.xmlkernel-bio-converter/run_local.bat
💤 Files with no reviewable changes (2)
- helm/converters/README.md
- kernel-bio-converter/Dockerfile
bbbf0f1 to
4fcd252
Compare
Applies all file changes from mosip/converters v1.3.0 tag relative to upstream/develop, including: - pom.xml: Spring Boot and dependency version updates - Dockerfile updates - Helm chart (Chart.yaml, values.yaml, README.md) updates - Deploy scripts (copy_cm.sh, delete.sh, install.sh, restart.sh) - ConvertController.java updates - NOTICE and README.md updates - Removed redundant license files already covered by NOTICE Signed-off-by: kameshsr <kameshsr1338@gmail.com>
4fcd252 to
a6905ad
Compare
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
kernel-bio-converter/pom.xml (1)
77-79:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSonar exclusions revert to incorrect class name.
Past reviews identified that
AuditManagerBootApplication.javashould beKernelBioConverterApplication.java(this module's actual Spring Boot main class), and this was marked as addressed. However, the current code has reverted back toAuditManagerBootApplication.java. Note that line 315 correctly excludesKernelBioConverterApplication.javain JaCoCo configuration, creating an inconsistency.🔧 Proposed fix
- <sonar.exclusions>**/dto/**,**/constant/**,**/config/**,**/httpfilter/**,**/cache/**,**/entity/**,**/model/**,**/exception/**,**/repository/**,**/request/**,**/spi/**,**/proxy/**,**/AuditManagerBootApplication.java</sonar.exclusions> - <sonar.coverage.exclusions>**/dto/**,**/constant/**,**/config/**,**/httpfilter/**,**/cache/**,**/entity/**,**/model/**,**/exception/**,**/repository/**,**/request/**,**/spi/**,**/proxy/**,**/AuditManagerBootApplication.java</sonar.coverage.exclusions> + <sonar.exclusions>**/dto/**,**/constant/**,**/config/**,**/httpfilter/**,**/cache/**,**/entity/**,**/model/**,**/exception/**,**/repository/**,**/request/**,**/spi/**,**/proxy/**,**/KernelBioConverterApplication.java</sonar.exclusions> + <sonar.coverage.exclusions>**/dto/**,**/constant/**,**/config/**,**/httpfilter/**,**/cache/**,**/entity/**,**/model/**,**/exception/**,**/repository/**,**/request/**,**/spi/**,**/proxy/**,**/KernelBioConverterApplication.java</sonar.coverage.exclusions>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kernel-bio-converter/pom.xml` around lines 77 - 79, The Sonar exclusions reverted to the wrong main-class name; update the sonar.exclusions and sonar.coverage.exclusions entries in pom.xml to reference KernelBioConverterApplication.java instead of AuditManagerBootApplication.java (and remove any lingering AuditManagerBootApplication.java tokens under sonar.* properties) so they match the JaCoCo exclusion for KernelBioConverterApplication.java; ensure sonar.cpd.exclusions remains unchanged unless it also contains the wrong class name.
🧹 Nitpick comments (1)
kernel-bio-converter/run_local.bat (1)
1-1: 💤 Low valueBatch file uses Unix line endings.
Static analysis detected LF-only line endings, which can cause parsing issues with the Windows batch interpreter due to GOTO/CALL label parsing failures. Batch files should use CRLF line endings.
🔧 How to fix
Convert the file to CRLF line endings using:
- Your editor's line-ending setting (usually in the status bar)
dos2unixtool (reversed mode)- Git configuration:
git config core.autocrlf true- Or configure
.gitattributeswith:*.bat text eol=crlf🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kernel-bio-converter/run_local.bat` at line 1, The batch file run_local.bat currently uses LF-only (Unix) line endings which can break Windows batch parsing; convert run_local.bat to use CRLF line endings so the java launch command (the long "java -Dloader.path=... -jar target/kernel-bio-converter-1.3.0-SNAPSHOT.jar" line) is parsed correctly by cmd.exe. Fix by updating the file's line endings in your editor to CRLF, or run a conversion tool (e.g., use dos2unix in reverse or an equivalent CRLF conversion), or set Git to handle .bat files as CRLF (git config core.autocrlf true or add "*.bat text eol=crlf" to .gitattributes) and recommit the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 62-75: The documentation incorrectly references error code
MOS-CNV-004 for unknown target formats; update the target-format validation
reference to MOS-CNV-002 so it matches IConverterApi's documented codes. Edit
the description in AGENTS.md (the step sequence describing ConvertController →
IConverterApi → ConverterServiceImpl) and replace MOS-CNV-004 with MOS-CNV-002
for the "Decode target format string → TargetFormatCode enum" step, ensuring
consistency with IConverterApi's MOS-CNV-001/002/003 mapping.
- Around line 100-102: Update the Javadoc on IConverterApi
(kernel-bio-converter/src/main/java/io/mosip/kernel/bio/converter/service/IConverterApi.java)
to correctly document the full range of ConversionException error codes used by
ConverterErrorCode and ConversionExceptionAdvice: include MOS-CNV-001 through
MOS-CNV-012 and MOS-CNV-500 (or explicitly mark the list as non-exhaustive).
Locate the Javadoc on the IConverterApi interface (search for the existing
mention of MOS-CNV-001/002/003/500) and expand the documented error codes or add
a clarifying note that the list is illustrative, ensuring it matches AGENTS.md
and the ConverterErrorCode enum.
In `@kernel-bio-converter/pom.xml`:
- Around line 81-85: The pom currently references unreleased snapshot properties
(kernel.bom.version, kernel.core.version, kernel.auth.adapter.version,
kernel.logger.logback.version, mosip.biometrics.util.version all set to
1.4.0-SNAPSHOT) which cannot be resolved by CI; either publish the
1.4.0-SNAPSHOT artifacts to your snapshot repository or revert these properties
to the latest published release versions (e.g., the previous stable version) so
Maven can resolve dependencies; update the properties in the pom.xml accordingly
and re-run the build to confirm resolution.
- Line 8: POM version is 1.4.0-SNAPSHOT but run_local.bat still references
kernel-bio-converter-1.3.0-SNAPSHOT.jar; update the run_local.bat invocation to
match the POM version (kernel-bio-converter-1.4.0-SNAPSHOT.jar) or change the
script to use a non-versioned/wildcard pattern (e.g.,
kernel-bio-converter-*.jar) so the JAR name and the <version> tag stay
consistent; locate the JAR reference in run_local.bat and replace the hardcoded
filename accordingly.
---
Duplicate comments:
In `@kernel-bio-converter/pom.xml`:
- Around line 77-79: The Sonar exclusions reverted to the wrong main-class name;
update the sonar.exclusions and sonar.coverage.exclusions entries in pom.xml to
reference KernelBioConverterApplication.java instead of
AuditManagerBootApplication.java (and remove any lingering
AuditManagerBootApplication.java tokens under sonar.* properties) so they match
the JaCoCo exclusion for KernelBioConverterApplication.java; ensure
sonar.cpd.exclusions remains unchanged unless it also contains the wrong class
name.
---
Nitpick comments:
In `@kernel-bio-converter/run_local.bat`:
- Line 1: The batch file run_local.bat currently uses LF-only (Unix) line
endings which can break Windows batch parsing; convert run_local.bat to use CRLF
line endings so the java launch command (the long "java -Dloader.path=... -jar
target/kernel-bio-converter-1.3.0-SNAPSHOT.jar" line) is parsed correctly by
cmd.exe. Fix by updating the file's line endings in your editor to CRLF, or run
a conversion tool (e.g., use dos2unix in reverse or an equivalent CRLF
conversion), or set Git to handle .bat files as CRLF (git config core.autocrlf
true or add "*.bat text eol=crlf" to .gitattributes) and recommit the file.
🪄 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: f36c55bf-2fe8-4b58-a90a-e375f8cbc04f
📒 Files selected for processing (19)
AGENTS.mdNOTICEREADME.mdTHIRD-PARTY-NOTICESdeploy/copy_cm.shdeploy/delete.shdeploy/install.shdeploy/restart.shhelm/converters/README.mdkernel-bio-converter/Dockerfilekernel-bio-converter/pom.xmlkernel-bio-converter/run_local.batkernel-bio-converter/src/main/java/io/mosip/kernel/bio/converter/controller/ConvertController.javalicenses/Apache-2.0.txtlicenses/BSD-3-Clause.txtlicenses/EPL-2.0.txtlicenses/MIT.txtlicenses/MPL-2.0.txtlicenses/NOTICE
💤 Files with no reviewable changes (9)
- licenses/MPL-2.0.txt
- licenses/MIT.txt
- licenses/Apache-2.0.txt
- licenses/EPL-2.0.txt
- THIRD-PARTY-NOTICES
- licenses/BSD-3-Clause.txt
- kernel-bio-converter/Dockerfile
- helm/converters/README.md
- licenses/NOTICE
✅ Files skipped from review due to trivial changes (3)
- NOTICE
- kernel-bio-converter/src/main/java/io/mosip/kernel/bio/converter/controller/ConvertController.java
- README.md
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
…ting Previously only MOS-CNV-001, -002, -003, and -500 were listed. Added MOS-CNV-004 through MOS-CNV-012 with descriptions matching ConverterErrorCode enum entries, as flagged by coderabbitai. Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
kernel-bio-converter/pom.xml (1)
77-79:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSonar exclusions reference wrong main class - regression of previously fixed issue.
Lines 77-78 exclude
AuditManagerBootApplication.java, but this module's main class isKernelBioConverterApplication.java(as correctly configured in JaCoCo exclusions at line 316). This violates the coding guideline requiring the main application class to be excluded from both JaCoCo and SonarCloud metrics, and creates inconsistency within the same pom file.This issue was previously raised and marked as addressed in commits 41f64a8 to bbbf0f1, but has reappeared in the current code (likely due to the release-1.3.x merge). The fix must be reapplied.
🔧 Reapply the fix
- <sonar.exclusions>**/dto/**,**/constant/**,**/config/**,**/httpfilter/**,**/cache/**,**/entity/**,**/model/**,**/exception/**,**/repository/**,**/request/**,**/spi/**,**/proxy/**,**/AuditManagerBootApplication.java</sonar.exclusions> - <sonar.coverage.exclusions>**/dto/**,**/constant/**,**/config/**,**/httpfilter/**,**/cache/**,**/entity/**,**/model/**,**/exception/**,**/repository/**,**/request/**,**/spi/**,**/proxy/**,**/AuditManagerBootApplication.java</sonar.coverage.exclusions> + <sonar.exclusions>**/dto/**,**/constant/**,**/config/**,**/httpfilter/**,**/cache/**,**/entity/**,**/model/**,**/exception/**,**/repository/**,**/request/**,**/spi/**,**/proxy/**,**/KernelBioConverterApplication.java</sonar.exclusions> + <sonar.coverage.exclusions>**/dto/**,**/constant/**,**/config/**,**/httpfilter/**,**/cache/**,**/entity/**,**/model/**,**/exception/**,**/repository/**,**/request/**,**/spi/**,**/proxy/**,**/KernelBioConverterApplication.java</sonar.coverage.exclusions>As per coding guidelines: "Exclude
constant/,config/,dto/,exception/, and the main application class from JaCoCo and SonarCloud code coverage metrics."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kernel-bio-converter/pom.xml` around lines 77 - 79, The Sonar exclusions mistakenly reference AuditManagerBootApplication.java instead of this module's main class; update the sonar.exclusions and sonar.coverage.exclusions entries to exclude KernelBioConverterApplication.java (matching the JaCoCo exclusion for KernelBioConverterApplication), replacing any AuditManagerBootApplication.java occurrences so the main application class is consistently excluded from both SonarCloud and JaCoCo metrics; leave sonar.cpd.exclusions as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@kernel-bio-converter/pom.xml`:
- Around line 77-79: The Sonar exclusions mistakenly reference
AuditManagerBootApplication.java instead of this module's main class; update the
sonar.exclusions and sonar.coverage.exclusions entries to exclude
KernelBioConverterApplication.java (matching the JaCoCo exclusion for
KernelBioConverterApplication), replacing any AuditManagerBootApplication.java
occurrences so the main application class is consistently excluded from both
SonarCloud and JaCoCo metrics; leave sonar.cpd.exclusions as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7926b87b-4ebe-4aa6-bfbb-2d861382ada5
📒 Files selected for processing (1)
kernel-bio-converter/pom.xml
Summary by CodeRabbit
Documentation
Chores