Skip to content
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.

Sign up for GitHub

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

Jump to bottom

OP-1197 | Validate ward Inventory #1456

Merged
merged 26 commits into from
Jan 14, 2025
Merged

OP-1197 | Validate ward Inventory #1456

merged 26 commits into from
Jan 14, 2025

Conversation

JantBogard
Copy link
Contributor

@JantBogard JantBogard commented Nov 20, 2024 •
edited by jira bot
Loading

See OP-1197

dbmalkovsky
dbmalkovsky previously requested changes Nov 20, 2024
View reviewed changes
Copy link
Collaborator

@dbmalkovsky dbmalkovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comments inline.

@mwithi mwithi mentioned this pull request Nov 21, 2024
Merged
@dbmalkovsky
Copy link
Collaborator

@JantBogard please resolve the conflicts; thanks

JantBogard and others added 5 commits December 9, 2024 09:05
@JantBogard @mwithi
Update src/main/java/org/isf/medicalinventory/manager/MedicalInventor…
f01faa7 
…yManager.java

Co-authored-by: Alessandro Domanico <alessandro.domanico@yahoo.it>
@JantBogard @mwithi
Update src/main/java/org/isf/medicalinventory/manager/MedicalInventor…
5415bf9 
…yManager.java

Co-authored-by: Alessandro Domanico <alessandro.domanico@yahoo.it>
@JantBogard @mwithi
Update src/main/java/org/isf/medicalinventory/manager/MedicalInventor…
1d9750e 
…yManager.java

Co-authored-by: Alessandro Domanico <alessandro.domanico@yahoo.it>
@JantBogard
Applying suggestion for change
51f2974
@JantBogard
Fix test coverage
07881cd
@JantBogard JantBogard requested a review from mwithi December 12, 2024 13:23
@mwithi
Copy link
Member

mwithi commented Jan 8, 2025

It seems the coverage is ok now for the new methods.

@dbmalkovsky is the code ok now?
@ArnaudFofou please it seems that validateMedicalInventoryRow method is present in both MedicalInventoryManager and MedicalInventoryManagerRow of course with different code and purpose, but a better naming would be advisable...

@ArnaudFofou
Copy link
Contributor

It seems the coverage is ok now for the new methods.

@dbmalkovsky is the code ok now? @ArnaudFofou please it seems that validateMedicalInventoryRow method is present in both MedicalInventoryManager and MedicalInventoryManagerRow of course with different code and purpose, but a better naming would be advisable...

of course I agree with you

mwithi
mwithi approved these changes Jan 14, 2025
View reviewed changes
Copy link
Member

@mwithi mwithi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and looks ok. Let's move on so we can complete it with the confirmation step and test again the whole feature. Thanks!!

@mwithi mwithi merged commit 43581de into informatici:develop Jan 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwithi mwithi mwithi approved these changes

@dbmalkovsky dbmalkovsky Awaiting requested review from dbmalkovsky

@ArnaudFofou ArnaudFofou Awaiting requested review from ArnaudFofou

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JantBogard @dbmalkovsky @mwithi @ArnaudFofou