search

lbruun-net / Pre-Liquibase

Spring Boot add-on to Liquibase
Apache License 2.0
49 stars 9 forks source link

Bump Spring Boot 2.7.0 #6

Closed zorglube closed 2 years ago

zorglube commented 2 years ago

@lbruun : Feel free to refuse part of the PR and to tell me what you don't like.

lbruun commented 2 years ago

Hi @zorglube. Thank you for our PR.

I think you've (accidentally?) reformatted a lot - if not all - of the source code files. This makes it hard to tell what your real intention is. Also the reformatting in itself is not for the better. For example, all indentions within comment sections has been removed by your formatter.

Just a hint: When contributing to FOSS projects you generally have to accept whatever code formatting convention is already used. The only way maintainers of such project would accept a PR which changes code formatting would be if you are correcting an inconsistency (for example one file not being formatted the same way as the other files in the project).

I hope this hasn't put you off from contributing again. You are most welcome.

lbruun commented 2 years ago

Btw: I've tested the existing released version of Pre-Liquibase, v1.1.1, with Spring Boot 2.7. It works fine.

I would think it is better to keep the deps for Pre-Liquibase at a conservative level, i.e. be one or two minor versions behind the currently released version of Spring Boot. At the moment Pre-Liquibase has a POM which uses Spring Boot 2.5 which means it is usable with Spring Boot 2.5, 2.6 and 2.7.

zorglube commented 2 years ago

Hi @zorglube. Thank you for our PR.

I think you've (accidentally?) reformatted a lot - if not all - of the source code files. This makes it hard to tell what your real intention is. Also the reformatting in itself is not for the better. For example, all indentions within comment sections has been removed by your formatter.

I'll manage that, by the way, do you have a fomatter.xml that you can add to the project ?

Just a hint: When contributing to FOSS projects you generally have to accept whatever code formatting convention is already used. The only way maintainers of such project would accept a PR which changes code formatting would be if you are correcting an inconsistency (for example one file not being formatted the same way as the other files in the project).

Yes you're right.

I hope this hasn't put you off from contributing again. You are most welcome.

No don't worry ;-)

zorglube commented 2 years ago

@lbruun : I think I'll make a rework on the PR and split the difference changes I made to help you review.

By the way, I made a test on the Database object, it it implement Closeable it works way better. So I filed a issue on the Liquibase GitHub.

lbruun commented 2 years ago

Closing this one as support for Spring Boot 2.7 is already present.

zorglube commented 2 years ago

Don't you want to move to JUnit 5 ? It was also included in this PR.

lbruun commented 2 years ago

Don't you want to move to JUnit 5 ? It was also included in this PR.

It is already at JUnit 5.

zorglube commented 2 years ago

How ok, I missed that, sorry.