openmrs-core icon indicating copy to clipboard operation
openmrs-core copied to clipboard

TRUNK-4988: Convenience method to allow change the configurations more easily

Open kdaud opened this issue 3 years ago • 2 comments

Description of what I changed

Convenience method to allow change the configurations more easily

Issue I worked on

see https://issues.openmrs.org/browse/TRUNK-4988

Checklist: I completed these to help reviewers :)

  • [ x] 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 --amend

  • [ ] I 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 --amend

  • [ x] I ran mvn clean package right before creating this pull request and added 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.

  • [ x] My pull request is based on the latest changes of the master branch.

    No? Unsure? -> execute command git pull --rebase upstream master

kdaud avatar Apr 29 '22 03:04 kdaud

Have you made an attempt to find what those TODO comment are / were and what the ticket actually asks to fix?

@ibacher I went through the file and got to find out the line numbers changed and based on the ticket description, I get to understand the ticket requires using Spring's method injection to set the dependencies for the message service.

Am not so familiar with spring method injection but my initial work is to add a bean in the applicationContext-service.xml so that the Message service dependencies are set via lookup-methods provided by the spring injection mechanism.

Could it be that the TODO is no-longer existing as of now? or I needed to approach it differently?

kdaud avatar May 01 '22 00:05 kdaud

@kdaud They're still there. They've just moved to lines 668 and 678. The first comment is a bit unfortunate because I think it directed you to the wrong place.

With Spring, we already have a Spring context created and we should basically never create a new one.

ibacher avatar May 04 '22 15:05 ibacher