Add LiquibasePreparer.forFile
#98
Conversation
|
@tomix26 what do you think of this change? |
|
Thanks for your contribution. I'll have to look at it first and verify the changes. I'll do it as soon as I can, I promise. |
|
There are some compilation errors, so you can check them out and fix it. Thanks in advance! |
src/main/java/io/zonky/test/db/postgres/embedded/LiquibasePreparer.java
Outdated
Show resolved
Hide resolved
| liquibase.update(contexts); | ||
|
|
||
| // Try and commit the changes. | ||
| connection.commit(); |
There was a problem hiding this comment.
Is this change necessary? I would prefer to keep it as it was. I guess the calling of liquibase.update(contexts) should commit all the changes, right? And the closing logic is already implemented in the finally block.
There was a problem hiding this comment.
Unconditionally rolling back in the finally seems incorrect. Closing without committing seems risky, so I added commit + close + set to null
There was a problem hiding this comment.
You're right that the previous rolling back logic was incorrect. I've replaced it with a try-with-resources block.
|
|
||
| import static org.junit.Assert.assertEquals; | ||
|
|
||
| public class LiquibasePreparerFileContextTest { |
There was a problem hiding this comment.
Thanks for the tests. Now it's much better 👍
Like `forClasspathLocation` but with a `FileSystemResourceAccessor`
|
Thanks again for the pull request. I've just tweaked a few details and squashed the commits into a single one for better clarity. And I'm going to release it in the next version. |
Like
forClasspathLocationbut with aFileSystemResourceAccessor