-
Notifications
You must be signed in to change notification settings - Fork 13
Make mongo-hibernate.jar a Java module
#149
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
base: main
Are you sure you want to change the base?
Conversation
3856819 to
332f624
Compare
| tasks.withType<JavaCompile>().configureEach { | ||
| options.compilerArgs.addAll(listOf("-Xlint:all", "-Werror")) | ||
| options.compilerArgs.addAll( | ||
| listOf("-Xlint:all", "-Xlint:-requires-automatic", "-Xlint:-requires-transitive-automatic", "-Werror")) |
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.
It seems fine to declare requires and requires-transitive for automatic modules, as long as those modules get their names specified via the Automatic-Module-Name JAR manifest attribute. See https://dev.java/learn/modules/automatic-module/#depending-on-automatic-modules for more details.
332f624 to
7b850d2
Compare
| useJavaOutput() | ||
| packageName("com.mongodb.hibernate.internal") | ||
| documentation.set( | ||
| "Generated by the <a href=\"https://github.com/gmazzo/gradle-buildconfig-plugin\">BuildConfig</a> plugin.\n\n@hidden") |
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 failed to exclude out generated code using -XepExcludedPaths - Error Prone seem to ignore this argument. So, to prevent it from complaining, I had to add the summary documentation line it complains about.
| addBooleanOption("Werror", false) | ||
| // TODO-HIBERNATE-129 addStringOption("-link-modularity-mismatch", "info") |
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.
We can't enable -Werror because of the single warning we have that can only be disabled in Java SE 18+.
| disableWarningsInGeneratedCode = true | ||
| // Error Prone does not understand the `@hidden` standard tag | ||
| disable("InvalidBlockTag") | ||
| disable("AssignmentExpression") |
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 upgraded the Error Prone, and it now has the AssignmentExpression check enabled by default. We don't have a problem with people using such expressions lightly, but they can be useful in some situations. We already have a single instance of using an assignment expression in the codebase.
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.
We no longer need this annotation, as now we can actually use sealed classes without the limitation of having to have all permits in the same package as the sealed class.
|
I realized that I should add some |
e6010cb to
de745c4
Compare
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 copied the canonical configuration file for logback from https://logback.qos.ch/manual/configuration.html (we were using the legacy one), and updated it with the changes we had.
| connection.set("scm:https://github.com/mongodb/mongo-hibernate.git") | ||
| developerConnection.set("scm:https://github.com/mongodb/mongo-hibernate.git") | ||
| connection.set("scm:git:https://github.com/mongodb/mongo-hibernate.git") | ||
| developerConnection.set("scm:git:https://github.com/mongodb/mongo-hibernate.git") |
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.
The format is scm:[provider]:[provider_specific] according to https://maven.apache.org/pom.html#SCM.
|
Talked with @vbabanin, his suggestions:
|
HIBERNATE-52
…umentation HIBERNATE-52
HIBERNATE-52
d884419 to
d8c4552
Compare
a1c0265 to
3c745d9
Compare
HIBERNATE-52
3c745d9 to
e15c18e
Compare
| <forkCount>1</forkCount> | ||
| <reuseForks>true</reuseForks> | ||
| <enableAssertions>true</enableAssertions> | ||
| <useModulePath>true</useModulePath> |
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 used useModulePath in both maven-surefire-plugin, maven-failsafe-plugin, and put the tests in a test module. This way we know the tests use the module path:
@vbabanin, thank you for the information about the
@hiddenstandard tag!@hiddenin a separate commit 70d7bee to make reviewing simpler.src/main/java/module-info.javais correct.I also upgraded Gradle to 9.2.1 (fortunately, it was trivial as the automatic upgrade worked just fine) and Error Prone (to the latest version that still works for us).
The only way I see of completely removing the non-exported program elements from the API documentation is to move all those packages to a newcom.mongodb.hibernate.internalmodule, whichexports ... to com.mongodb.hibernate. This requires declaring incom.mongodb.hibernatethatrequires transitive com.mongodb.hibernate.internal, and also requires publishing the artifact for thecom.mongodb.hibernate.internalmodule. Producing multiple Java modules also means using a multi-project Gradle build.HIBERNATE-52