-
Notifications
You must be signed in to change notification settings - Fork 0
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
SD-676 EBL-ICS2 Changes Conditional Validations #237
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here. CI Failure Feedback 🧐
✨ CI feedback usage guide:The CI feedback tool (
In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:
where Configuration options
See more information about the |
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.
out-dated comment; never mind
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.
On a second thought, after checking Sonar as well:
Regarding the Java part: please add empty lines between the methods and format EBLChecks.java
so it is easier to read. For the rest, I think it looks fine.
I assume Preetam checks if the checks make sense 😉
Almost forgot the mention: but do you also add unit tests?
The duplicated checks should also be fixed.
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.
As discussed, use the existing available methods in JsonAttribute to validate the attributes
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EBLChecks.java
Outdated
Show resolved
Hide resolved
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EBLChecks.java
Outdated
Show resolved
Hide resolved
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EBLChecks.java
Outdated
Show resolved
Hide resolved
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EBLChecks.java
Outdated
Show resolved
Hide resolved
Thanks @preetamnpr for the comments, I will change this to the existing methods. Will add unit tests too. |
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.
Looking good. Just wondering if you forgot to commit the unit tests. If not, you don't need to add the dependency.
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EBLChecks.java
Outdated
Show resolved
Hide resolved
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.
Nice! Got 83.2% coverage in this PR 👍🏻
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 changes looks fine.
PR Type
enhancement, tests
Description
EBLChecks.java
to ensure data integrity based on specific conditions.EXEMPT_PACKAGE_CODES
inEblDatasets.java
to support new validation logic.ebl-api-3.0.0-request.json
to include new fields and scenarios for testing.Changes walkthrough 📝
EBLChecks.java
Add conditional validation checks for EBL fields
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EBLChecks.java
conditions.
EblDatasets.java
Add exempt package codes dataset
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblDatasets.java
EXEMPT_PACKAGE_CODES
dataset.ebl-api-3.0.0-request.json
Update JSON request example with new fields
ebl/src/main/resources/standards/ebl/messages/ebl-api-3.0.0-request.json
ladings.