TRUNK-5903: Switching from hibernate mappings to annotations-Role #4293
Conversation
|
@mherman22 Got some failing tests at Error: Tests run: 29, Failures: 0, Errors: 1, Skipped: 2, Time elapsed: 2.353 s <<< FAILURE! - in org.openmrs.hl7.HL7ServiceTest |
@tendomart i am looking into that. Feel free to read through this talk post though -> https://talk.openmrs.org/t/switching-from-xml-mappings-to-annotations/39277/1 |
mherman22
force-pushed
the
TRUNK-5903
branch
3 times, most recently
from
6bf1299 to
b364c61
Compare
There was a problem hiding this comment.
Thanks @mherman22! A few small things, but this seems like the right direction...
|
looks to be a failure on the entire core as of this commit --> 7e9a3b6 |
|
It's a flaky test... Or at least, the commit you point to only updates the README.md file, so I'm not certain how that could change test results. |
This test passes when i build locally. |
mherman22
force-pushed
the
TRUNK-5903
branch
2 times, most recently
from
2d80f75 to
e582a0f
Compare
mherman22
force-pushed
the
TRUNK-5903
branch
2 times, most recently
from
0b7abab to
3bd5c51
Compare
mherman22
changed the title
| @Transient | ||
| @Field | ||
| private String name; |
There was a problem hiding this comment.
So I'm still kind of curious why we need this?
There was a problem hiding this comment.
so this was to make sure that the name field doesnot get persisted in the role table where we only require role,uuid and description.
There was a problem hiding this comment.
Right, but Role now extends BaseOpenmrsObject which doesn't have that field any more right?
There was a problem hiding this comment.
(I do think this should implement the OpenmrsMetadata interface, just with getName() and setName() modifying the role).
There was a problem hiding this comment.
Right, but Role now extends
BaseOpenmrsObjectwhich doesn't have that field any more right?
oh yeah, my bad
(I do think this should implement the
OpenmrsMetadatainterface, just withgetName()andsetName()modifying the role).
remember this? -> https://talk.openmrs.org/t/switching-from-xml-mappings-to-annotations/39277/4?u=mherman22
There was a problem hiding this comment.
Well, sure. What I actually care about is ensuring Role implements the Auditable and Retireable interfaces.
There was a problem hiding this comment.
awesome, so what i gather here is that role should implement OpenmrsMetadata interface and that this ticket -> https://openmrs.atlassian.net/browse/RESTWS-910 is not required
mherman22
force-pushed
the
TRUNK-5903
branch
4 times, most recently
from
c5f4f28 to
0c9c811
Compare
|
@mherman22 did you see the merge conflicts? |
mherman22
force-pushed
the
TRUNK-5903
branch
2 times, most recently
from
2be2700 to
16a9173
Compare
|
tl;dr our action detected no activity on this PR and will close it in 30 days if the stale label is not removed. OpenMRS welcomes your contribution! It means a lot to us that you want to contribute to equity in healthcare! This PR has not seen any activity in the last 5 months. That is why we wanted to check whether you are still working on it or need assistance from our side. If you do not have time to continue the work or have moved on you don’t need to do anything. We will automatically close the PR in 30 days. We hope to see you back soon :) |
|
tl;dr closing this PR since it has not seen any activity in the last 6 months. OpenMRS welcomes your contribution! It means a lot to us that you want to contribute to equity in healthcare! We certainly do not want to discourage you from contributing. We do need to be honest in that OpenMRS has limited resources for reviewing PRs. Your PR has not seen any activity in the last 6 months. This is why we have decided to close this PR for now. This allows us OpenMRS reviewers to focus our limited time to review all other PRs in a timely and professional manner. Please feel free to reassign yourself to the issue you worked on in our JIRA when you have time to focus on it. After that reopen a new PR and we will be glad to work with you to get your contribution merged. Thank you very much for your help and understanding :) |
Description of what I changed
I have changed the role domain from using hibernate xml mappings to annotations.
Also ensured the role class extends the extends BaseOpenmrsObject but also implements the retireable and Auditable interfaces.
I made the name attribute transient so that it doesn't get persisted.
Issue I worked on
see https://issues.openmrs.org/browse/TRUNK-5903
Checklist: I completed these to help reviewers :)
My IDE is configured to follow the code style of this project.
No? Unsure? -> configure your IDE, format the code and add the changes with
git add . && git commit --amendI have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)
No? -> write tests and add them to this commit
git add . && git commit --amendI ran
mvn clean packageright before creating this pull request andadded all formatting changes to my commit.
No? -> execute above command
All new and existing tests passed.
No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.
My pull request is based on the latest changes of the master branch.
No? Unsure? -> execute command
git pull --rebase upstream master