See gh-10739
Comment From: pivotal-issuemaster
@jbertram Please sign the Contributor License Agreement!
Click here to manually synchronize the status of this Pull Request.
See the FAQ for frequently asked questions.
Comment From: pivotal-issuemaster
@jbertram Thank you for signing the Contributor License Agreement!
Comment From: kmandalas
@jbertram I see some minor code Formatting violations in CI build, for files ArtemisConnectionFactoryFactory.java and rtemisAutoConfigurationTests.java. I guess can be resolved by running gradle format
Comment From: jbertram
@kmandalas the CI build issues have been resolved.
Comment From: kmandalas
@pivotal-issuemaster @spring-issuemaster can we proceed with the merge?
Comment From: philwebb
@kmandalas We need to wait until we've created a 2.4.x branch before we can merge this.
Comment From: snicoll
I suspect it was flagged with merge amendments as we don't handle deprecated properties this way (don't worry we can take care of it while polishing).
Comment From: snicoll
@jbertram thank you for your contribution. I've polished it to prevent users using the (now) deprecated host property to get their host/port configuration ignored (which is a smell of you having to add an empty broker-url property here). We now rather check if no broker url is set by the user and if a host is set we configure as we previously did. Of course, if a broker url is set, we'll always use that and I've added an extra test to validate that scenario.
Comment From: jbertram
@snicoll, sounds great! Thanks for the review and help with polishing. Feel free to @ me on anything in the future related to Artemis that you may have questions about.
Comment From: snicoll
Will do, thanks again for the great support Justin.