Spring boot upgrade 2.1.2.RELEASE #63
Conversation
There was a problem hiding this comment.
@AbhayChandel 👍 thanks for the PR. It looks great in general. Can you please have a look at the issue I found regarding the BOM and try to rework that. Everything else looks perfect to me.
Thank you so much!
| <groupId>javax.persistence</groupId> | ||
| <artifactId>javax.persistence-api</artifactId> | ||
| <version>2.2</version> |
There was a problem hiding this comment.
why did you remove the version? IMHO this is a mistake. A dependency in depencenyManagement without a version seems pointless to me. Or is this a miss-interpretation of the diffs?
Line 93 in d9d7403
There was a problem hiding this comment.
If that dependency comes in 2.2+ version via springs BOMs and we do not want to repeat it here, you should remove it entirely.
There was a problem hiding this comment.
Nice catch @hohwille , javax.persistence-api:2.2 is already defined in spring-boot-depenedencies:2.1.2.RELEASE. So I think it is better to remove
it completely from devon4j-bom. Removed and changes pushed to the PR.
| @@ -172,7 +173,7 @@ public void execute(Operation operation, List<String> configurations, String job | |||
| SpringApplication app = new SpringApplication(configurationClasses); | |||
|
|
|||
| // no (web) server needed | |||
| app.setWebEnvironment(false); | |||
| app.setWebApplicationType(WebApplicationType.NONE); | |||
There was a problem hiding this comment.
Fine fix. However, this reminds me of the fact that our batch still needs rework and polising as this is old fashion style code. But removing the spring XMLs and moving to a modern spring-boot only config is not in the scope of this PR, so all perfectly right.
There was a problem hiding this comment.
@AbhayChandel Thanks. Looking fine now. As we rely on an external dependencyManagement then, can you confirm this:
- we do not depend on this proprietary dependency anymore
org.hibernate.javax.persistence:hibernate-jpa-2.1-api? - instead we depend on
javax.persistence:javax.persistence-api:2.2, correct?
Yes, we are no more depending devon4j on |
This PR is for issue #12