-
-
Notifications
You must be signed in to change notification settings - Fork 216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add jooq module with maven #11107
Add jooq module with maven #11107
Conversation
bed27c6
to
b9a7721
Compare
b9a7721
to
ddf6d78
Compare
for the CI test, you can create a new dedicated config with what you need to test jooq Another idea would be to add it to mariadb, it should work: https://github.com/jhipster/jhipster-lite/blob/main/tests-ci/generate.sh#L232-L239 |
But the plugin connects to a database to introspect schema and generate classes depending on what tables exists. |
Here some steps in the GitHub CI:
|
I started to investigate on it but in fact the db needs to be up at compile time not when we start the app, so on step "Test: verify: " Regarding the workflow, I don't think we have this use case and I don't find a proper way to do it Here an example : https://github.com/fabienpuissant/jhipster-lite/actions/runs/11622943610/job/32369374084 |
cd0a545
to
771dfef
Compare
@@ -0,0 +1,140 @@ | |||
package tech.jhipster.lite.module.domain.jooqplugin; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't thinks this should be in module/domain
, this is too specif.
How about moving that in the jooq/domain
? (and remove the addition in JHipsterModule)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep + solve deplicate issue about database enum
.build() | ||
) | ||
.and() | ||
.mavenPlugins() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NB : gradle should be supported too, by configuration the gradle plugin. See https://www.jooq.org/doc/latest/manual/code-generation/codegen-gradle/
But this must be done in a second PR. Meanwhile you can require maven as a dependency of all jooq module in JooqModuleConfiguration to prevent them from being used with gradle.
<inputSchema>%s</inputSchema> | ||
</database> | ||
<target> | ||
<packageName>org.jooq.codegen</packageName> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this should rather be the packageName from properties (e.g. generates classes to com.my.company rather than org.jooq.codegen)
| mariadb.md | | ||
And I should have files in "src/main/docker" | ||
| mariadb.yml | | ||
And I should have files in "src/main/resources/config" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is already created by the spring-boot
module, so I guess you want have a finer-grained assertion like
And I should have files in "src/main/resources/config" | |
And I should have "jdbc:mariadb://localhost:3306/" in "src/main/resources/config/application.yml" |
b6fac3e
to
de6b01b
Compare
LGTM. @fabienpuissant : could you rebase the branch to fix conflicts? |
Head branch was pushed to by a user without write access
de6b01b
to
42678fa
Compare
ci test not relevant because need a database up to install with this jooq plugin, cannot test on mssql. Error to connect due to url error.
See #9235