TOMEE-1380 - add examples for liquibase/flyway usage #1525

jrxxjr · 2024-09-28T02:46:29Z

Added the example of the use Flyway and Liquibase tool, via programmatically and via maven plugin

…ically and via maven plugin

…matically and via maven plugin

Daniel-Dos

Very good, thanks.

jungm

Awesome PR, we can always need more examples that guide users! I've annotated a few minor things I'd like to see changed, then we can merge this IMO

examples/import-database-flyway-maven/README.adoc

examples/import-database-flyway-maven/pom.xml

examples/import-database-flyway/pom.xml

examples/import-database-liquibase-maven/pom.xml

examples/import-database-liquibase/.gitignore

examples/import-database-liquibase-maven/README.adoc

...e/src/main/java/org/apache/openejb/assembler/classic/migrate/database/ImportByLiquibase.java

.../src/test/java/org/apache/openejb/assembler/classic/migrate/database/liquibase/changelog.xml

jrxxjr · 2024-09-28T17:30:03Z

I saw the problem, and i am fixing it.

…the use Liquibase tool, via programmatically and via maven plugin

jungm

just a few more small changes from my side, most of which are cleaning up POMs and READMEs

examples/import-database-flyway-maven/pom.xml

examples/import-database-flyway/README.adoc

examples/import-database-flyway/pom.xml

examples/import-database-liquibase-maven/pom.xml

examples/import-database-liquibase/README.adoc

examples/import-database-liquibase/pom.xml

…the use Liquibase tool, via programmatically and via maven plugin

rzo1

Thanks for the PR. Please find some comments / feedback.

rzo1 · 2024-09-30T06:57:22Z

container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java

@@ -17,6 +17,70 @@

 package org.apache.openejb.assembler.classic;

+import static org.apache.openejb.util.Classes.ancestors;


Can we revert this change in import order? It doesn't actually contribute to the change set.

I did not understand correctly, what do you mean ?

The import ordering of the Assembler class was changed for no reason. I think, it was done by auto formatting of your IDE. Just restore the previous order of the import statements.

I will return the imports that are in the master branch.

container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java

rzo1 · 2024-09-30T06:59:14Z

...jb-core/src/main/java/org/apache/openejb/assembler/classic/EntityManagerFactoryCallable.java

@@ -122,8 +122,13 @@ public EntityManagerFactory call() throws Exception {
                emf.createEntityManager().close();
            }

+            /*


container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ImportSql.java

rzo1 · 2024-09-30T07:00:40Z

...ner/openejb-core/src/test/java/org/apache/openejb/assembler/classic/ImportSqlScriptTest.java

@@ -90,6 +101,11 @@ public static class Something {
        private long id;
    }

+    /*
+	 * The code below should no longer be used, because the ImportSql class, is no


This comment can be dropped.

examples/import-database-flyway-maven/README.adoc

rzo1 · 2024-09-30T07:03:23Z

examples/import-database-flyway/README.adoc

+			Flyway flyway = Flyway.configure().locations("filesystem:src/test/resources").dataSource(dataSource)
+					.cleanDisabled(false).load();
+
+			flyway.clean();


Might not be a good idea to suggest to use clean ;-) - if you do that in a productive app, you need to restore from backup.

Ok, i will change.

rzo1 · 2024-09-30T07:04:22Z

examples/import-database-flyway/pom.xml

+		<dependency>
+			<groupId>com.zaxxer</groupId>
+			<artifactId>HikariCP</artifactId>
+			<version>5.1.0</version>


Ok, i will change.

examples/import-database-liquibase-maven/pom.xml

…the use Liquibase tool, via programmatically and via maven plugin

rzo1

I only have two points:

(1) Please revert the changes in import order and any formatting in classes, which aren't changed. These Changes are still in the changeset.
(2) Please reduce the amount of copy paste comments. Duplicating the same comments about the deprecation of a class is Not necessary. IDE will highlight the deprecation anyway, so it is enough to have it in ImportSql itself.

Otherwise, looks good to me

jrxxjr · 2024-10-03T12:05:39Z

Responding to your observations:
(1) - I returned the order of the imports you requested, from the Assembler.java class, as it was in the master branch.
The code formatting is the Eclipse IDE standard, I didn't change that.
(2) - So i should only put comments in the ImportSql class, i don't need to put them in the javadoc in the ImportSqlScriptTest test class, nor in the coding of the Assember class ?

rzo1 · 2024-10-03T12:27:13Z

(1) Yep, I know it can happen easily. We don't have checkstyle in TomEE but changing import order or formatting at all due to auto formatting just makes the diff harder to read and doesn't bring much value. Therefore, I prefer to not touch it ;-)
(2) Yes.

jrxxjr · 2024-10-03T13:06:30Z

(1) - Wouldn't it be better to create a checkstyle in TomEE, so that anyone who changes the code, the formatting would always be forced on everyone, and this problem would not happen ?
(2) - Ok.

rzo1 · 2024-10-03T13:12:27Z

In a perfect world, enforcing style and providing formatting profiles for IntelliJ and Eclipse would be beneficial, yes.

But there are more sophisticated challenges atm, imho (one would need to start a discussion and find consensus about a common style, etc).

jrxxjr · 2024-10-03T13:15:53Z

In a perfect world, enforcing style and providing formatting profiles for IntelliJ and Eclipse would be beneficial, yes.

But there are more sophisticated challenges atm, imho (one would need to start a discussion and find consensus about a common style, etc).

Ok.

…the use Liquibase tool, via programmatically and via maven plugin

rzo1 · 2024-10-04T14:04:54Z

Thanks for the examples and the PR, @jrxxjr !!

jrxxjr added 5 commits September 15, 2024 16:39

TOMEE-1380 - Added the example of the use Flyway tool, via programmat…

7cdf8ed

…ically and via maven plugin

TOMEE-1380 - Correction of the documentation text.

5e3f0c7

TOMEE-1380 - Added the example of the use Liquibase tool, via program…

8434d7c

…matically and via maven plugin

TOMEE-1380 - Added the example of the use Liquibase tool, via program…

d3d3bea

…matically and via maven plugin

TOMEE-1380 - Added the example of the use Liquibase tool, via program…

c75b552

…matically and via maven plugin

Daniel-Dos approved these changes Sep 28, 2024

View reviewed changes

jungm requested changes Sep 28, 2024

View reviewed changes

jungm changed the title ~~Tomee 1380~~ TOMEE-1380 - add examples for liquibase/flyway usage Sep 28, 2024

jrxxjr added 2 commits September 29, 2024 08:05

TOMEE-1380 - Correction of the comments in Pull Request - Example of …

4994a10

…the use Liquibase tool, via programmatically and via maven plugin

TOMEE-1380 - Correction of the comments in Pull Request - Example of …

79bb85d

…the use Liquibase tool, via programmatically and via maven plugin

jrxxjr requested review from jungm and Daniel-Dos September 29, 2024 12:04

jungm requested changes Sep 29, 2024

View reviewed changes

jrxxjr added 2 commits September 29, 2024 09:43

TOMEE-1380 - Correction of the comments in Pull Request - Example of …

b61f40d

…the use Liquibase tool, via programmatically and via maven plugin

TOMEE-1380 - Correction of the comments in Pull Request - Example of …

122f126

…the use Liquibase tool, via programmatically and via maven plugin

jrxxjr requested a review from jungm September 29, 2024 12:56

rzo1 requested changes Sep 30, 2024

View reviewed changes

jrxxjr added 2 commits October 2, 2024 22:35

TOMEE-1380 - Correction of the comments in Pull Request - Example of …

e15f21b

…the use Liquibase tool, via programmatically and via maven plugin

TOMEE-1380 - Correction of the comments in Pull Request - Example of …

024348c

…the use Liquibase tool, via programmatically and via maven plugin

jrxxjr requested a review from rzo1 October 3, 2024 01:46

rzo1 approved these changes Oct 3, 2024

View reviewed changes

jrxxjr requested a review from rzo1 October 3, 2024 12:05

TOMEE-1380 - Correction of the comments in Pull Request - Example of …

b7306f8

…the use Liquibase tool, via programmatically and via maven plugin

rzo1 approved these changes Oct 3, 2024

View reviewed changes

jungm approved these changes Oct 3, 2024

View reviewed changes

Small fixes

cae2fde

rzo1 merged commit 3fe0c4f into apache:main Oct 4, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TOMEE-1380 - add examples for liquibase/flyway usage #1525

TOMEE-1380 - add examples for liquibase/flyway usage #1525

jrxxjr commented Sep 28, 2024

Daniel-Dos left a comment

jungm left a comment

jrxxjr commented Sep 28, 2024 via email •

edited

Loading

jungm left a comment

rzo1 left a comment

rzo1 Sep 30, 2024

jrxxjr Sep 30, 2024

rzo1 Sep 30, 2024

jrxxjr Sep 30, 2024

jrxxjr Oct 1, 2024

rzo1 Sep 30, 2024

rzo1 Sep 30, 2024

jrxxjr Sep 30, 2024

rzo1 Sep 30, 2024

jrxxjr Sep 30, 2024

rzo1 Sep 30, 2024

jrxxjr Sep 30, 2024

rzo1 left a comment

jrxxjr commented Oct 3, 2024

rzo1 commented Oct 3, 2024 •

edited

Loading

jrxxjr commented Oct 3, 2024

rzo1 commented Oct 3, 2024

jrxxjr commented Oct 3, 2024

rzo1 commented Oct 4, 2024

		@@ -17,6 +17,70 @@

		package org.apache.openejb.assembler.classic;

		import static org.apache.openejb.util.Classes.ancestors;

TOMEE-1380 - add examples for liquibase/flyway usage #1525

TOMEE-1380 - add examples for liquibase/flyway usage #1525

Conversation

jrxxjr commented Sep 28, 2024

Daniel-Dos left a comment

Choose a reason for hiding this comment

jungm left a comment

Choose a reason for hiding this comment

jrxxjr commented Sep 28, 2024 via email • edited Loading

jungm left a comment

Choose a reason for hiding this comment

rzo1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rzo1 left a comment

Choose a reason for hiding this comment

jrxxjr commented Oct 3, 2024

rzo1 commented Oct 3, 2024 • edited Loading

jrxxjr commented Oct 3, 2024

rzo1 commented Oct 3, 2024

jrxxjr commented Oct 3, 2024

rzo1 commented Oct 4, 2024

jrxxjr commented Sep 28, 2024 via email •

edited

Loading

rzo1 commented Oct 3, 2024 •

edited

Loading