Thanks @mogoodrich,
Yes, as far as changes related to the data filter module were made.
@wyclif, I hope that we have no interference when these changes(annotations on Person/Patient) are reverted.
1 Like
A work around with hibernate mappings on the side of the data filter module is to be made before we can revert the introduced annotations on the Person/Patient domains.
cc: @wyclif
Why should we revert from annotations for Person/Patient? We reverted for some of the other classes because they required adding dummy tables which we are not doing for Person and Patient
@mogoodrich there were issues of bad test data and not behavior changes after switching to annotations.
How about the change of behavior that led to this ticket? https://issues.openmrs.org/browse/TRUNK-5691
Yeah, thanks @dkayiwa that was one issue.
It could be argued that this is a small issue that doesnāt manifest itself in āreal lifeā much (though I havenāt done detailed analysis).
Bigger picture, though, it would seem best to me to have a consistent way to set up mappings, not annotations for Patient/Person but mapping files for everything else, so if thereās no longer a need to switch Patient/Person Iād strongly recommend reverting for consistency and to avoid the above the issue (and any other issues we may have missed). No point in adding something that could introduce issues if thereās not a compelling need.
Take care, Mark
@wyclif, I hope we can safely go ahead to revert from annotations for Person/Patient if this has no implications to the ongoing developments on the data filter module ?
Hi all, we are about to have the Platform 2.3.0 released by the 21st and from the PM call today, this will pave way for the pending Reference Application release. We hope that by the 17th January (latest) there are no issues pending soon after the requested changes/reverts are effected. Thank you all for your tireless efforts contributing to OpenMRS. 
Regards
3 Likes
Hi all, Iāll be reverting the following commits as a follow up on the changes introduced due to Patient/Person domain objects being annotated.
The last commit in the above sequence probably was addressing an issue that arose due to the Person/Patient domain objects being annotated. @bistenes, @mogoodrich , would reverting this, affect progress of developments that led to this? Thank you.
Kind regards.
Nathan
1 Like
@ruhanga I donāt think TRUNK-5675 is related to the Person/Patient domain object change and we wouldnāt want to revert itā¦ if Iām missing something and reverting the Person/Patient changes without reverting TRUNK-5675 causes issues, lmkā¦
Also, I donāt know if thereās any harm in keeping TRUNK-5685? Seems nice to keep that protected so subclasses can access it?
Thanks! Mark
Thanks @mogoodrich. Yes, reverting the annotation changes on Patient/Person affects the commits on accents (TRUNK-5675), I get failing tests related to the commit on this. Also the reverts on Patient/Person annotations causes failing tests related to TRUNK-5685 for the protected method.
A safe revert from annotations on Patient/Person domain object currently requires the above sequence of commits get reverted where all tests pass. Perhaps with a few tweaks we could keep the commit on TRUNK-5675.
@ruhanga I was doing some testing with the 2.3.x and couldnāt reproduce the issues you were seeing, but then I just tested with master and was getting some errors but ran out of time tonight, but will need to look more into itā¦
Reverting TRUNK-5685 isnāt a problem, but we will to keep TRUNK-5675ā¦
That being said, if you want to go ahead and revert it for now, feel free, just please reopen the ticket and flag me here so I can look into it further.
Thanks! Mark
1 Like
@mogoodrich, after building the branch 2.3.x with the reverted changes except for TRUNK-5675 in a windows and macOS environments, some tests related to TRUNK-5675 failed on windows. There is a fix that was not backported to the 2.3.x branch. I hope I can release the platform as soon as it is backported. Otherwise there are no other blockers and we can keep TRUNK-5675. Thank you.
@ruhanga backport has been applied, itās going through the build now, let me know if you notice any issues!
1 Like
I was going crazy about what annotations have to do with accents in strings.
I personally strongly believe that we should use annotations and start to move away from xml because xml masks bugs that would otherwise be exposed by annotations since they are part of the java language, also a lot of advancements and libraries of the language are implemented with annotations.
2 Likes
@wyclif , i personally agree with you.
My opinion was not to do reverts but rather to put in more effort to switch completely to annotations
I had thought that a switch to annotations would be obvious. But it turned out not to be so, basing on the side effects that we have experienced, and probably more would show up. Therefore, instead of annotations delaying the 2.3.0 release, i would bump them to 2.4.0 or even 3.0.0
3 Likes
Thank you @mogoodrich, @dkayiwa, @wyclif, @mozzy and @all for your thoughts and support. For now Iām going ahead to release the platform without the annotations for the purpose of consistency and not having to postponed this release.
However annotations on domain objects are more contemporary and convenient for use. This could be part of our GSoC projects and future releases.