add static analysis gradle plugins (spotless + palantir, errorprone + nullaway) #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jyemin
requested changes
Oct 10, 2024
NathanQingyangXu
force-pushed
the
HIBERNATE-5
branch
from
fe3ae67 to
922890c
Compare
1. update .editorconfig to align with palantir 2. remove usage of jspecify (mongo's nullness annotations suffice) 3. remove unnecessary comments
I don't think we should do this, as the driver's null-related annotations are not intended to be part of the driver API. They are just an implementation detail. |
reverted back |
rozza
approved these changes
Oct 21, 2024
| @@ -29,6 +33,11 @@ dependencies { | |||
| testImplementation(libs.junit.jupiter) | |||
|
|
|||
| testRuntimeOnly("org.junit.platform:junit-platform-launcher") | |||
|
|
|||
| errorprone(libs.nullaway) | |||
| api(libs.jspecify) | |||
There was a problem hiding this comment.
Even though this is documented as right, it seems wrong to have this as an api dependency - as its really for compile time only. 🤷
There was a problem hiding this comment.
I found the following statement at https://jspecify.dev/docs/using/
So let us stick to api scope for now and always keep it in mind to switch to lower visibility. Currently given almost zero experience on real usage, let us trust the doc. The switch cost should be minor in the future, if needed.
|
Palantir Intellij plugin is not well-maintained and latest plugin simply
doesn't work in latest Intellij. Yeah, we can invoke gradle task to
reformat automatically. But an .editorconfig ensures the least surprise so
he/she seldom needs to be bothered with deep diving to figure it out why CI
building failed.
…On Mon, Oct 21, 2024, 5:01 a.m. Ross Lawley ***@***.***> wrote:
***@***.**** approved this pull request.
LGTM!
Out of interest was the .editorconfig change a manual change or automatic
based on the formatting plugins?
------------------------------
In build.gradle.kts
<#2 (comment)>
:
> @@ -29,6 +33,11 @@ dependencies {
testImplementation(libs.junit.jupiter)
testRuntimeOnly("org.junit.platform:junit-platform-launcher")
+
+ errorprone(libs.nullaway)
+ api(libs.jspecify)
Even though this is documented as right, it seems wrong to have this as an
api dependency - as its really for compile time only. 🤷
—
Reply to this email directly, view it on GitHub
<#2 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB6UYAUDPBTVIVHVPTYCMPDZ4S7E3AVCNFSM6AAAAABPXWWUL2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOBRGM4DEMBWHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Good point on jspecify scope! It would be great to limit its visibility so
our client won't include it.
On Mon, Oct 21, 2024, 8:35 a.m. Nathan Xu ***@***.***>
wrote:
… Palantir Intellij plugin is not well-maintained and latest plugin simply
doesn't work in latest Intellij. Yeah, we can invoke gradle task to
reformat automatically. But an .editorconfig ensures the least surprise so
he/she seldom needs to be bothered with deep diving to figure it out why CI
building failed.
On Mon, Oct 21, 2024, 5:01 a.m. Ross Lawley ***@***.***>
wrote:
> ***@***.**** approved this pull request.
>
> LGTM!
>
> Out of interest was the .editorconfig change a manual change or
> automatic based on the formatting plugins?
> ------------------------------
>
> In build.gradle.kts
> <#2 (comment)>
> :
>
> > @@ -29,6 +33,11 @@ dependencies {
> testImplementation(libs.junit.jupiter)
>
> testRuntimeOnly("org.junit.platform:junit-platform-launcher")
> +
> + errorprone(libs.nullaway)
> + api(libs.jspecify)
>
> Even though this is documented as right, it seems wrong to have this as
> an api dependency - as its really for compile time only. 🤷
>
> —
> Reply to this email directly, view it on GitHub
> <#2 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AB6UYAUDPBTVIVHVPTYCMPDZ4S7E3AVCNFSM6AAAAABPXWWUL2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOBRGM4DEMBWHA>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
NathanQingyangXu
added a commit
that referenced
this pull request
Nov 4, 2024
NathanQingyangXu
added a commit
to NathanQingyangXu/mongo-hibernate
that referenced
this pull request
Nov 7, 2024
NathanQingyangXu
added a commit
that referenced
this pull request
Nov 14, 2024
* implement ConnectionProvider * change minimal MongoDB version to 6 * Update src/test/resources/hibernate.properties Co-authored-by: Jeff Yemin <[email protected]> * remove package-info.java in test folder; add copyright to package-info.java in main folder * improve Javadoc for MongoConnectionProvider per review comments * remove unnecessary codec setting stuff * simplify the dummy implementation of the three methods irrelevant to our usage * simplify MongoConnectionProvider per code review comments * remove exception package and move ConfigurationException to cfg package * add missing Javadocs for MongoDialect * add MQL acronym expansion in Javadoc * round #2 code review comments * change mininum mongodb version to v6.3 * use HibernateException instead of ConfigurationException.java * switch MongoDialect minimum db version to 6.0 * Update src/main/java/com/mongodb/hibernate/jdbc/MongoConnectionProvider.java Co-authored-by: Jeff Yemin <[email protected]> * deleted unused constructors in MongoDialect * add Javadoc for the root package-info.java * make more exception messages begin with uppercase * update various gradle plugin to latest versions * add sl4j logging dependencies and logback-test.xml config * fix a static check issue * change default constructor of MongoDialect to end up with null value; override getMinimumSupportedVersion() * some minor improvements * change as per Valentin's excellent code review comments * update SessionFactoryTests as per code review comments * update MongoDialect as per code review comments * update MongoConnectionProvider as per code review comments * delete .editorconfig file and move it to another separate PR * fix a static check style issue * resolve some code review comments * fix style checking issue * minor changes to improve Dialect min version usage * improve mentioning of MongoDB Java Driver in MongoDialect's Javadoc * add transient keyword to MongoClient in MongoConnectionProvider * Update src/main/java/com/mongodb/hibernate/jdbc/MongoConnectionProvider.java Co-authored-by: Valentin Kovalenko <[email protected]> * change as per code review comments * remove database checking logic * fix import style issue * change per some code review comments * add missing 'compileJava' gradle task in evergreen's static-check.sh --------- Co-authored-by: Jeff Yemin <[email protected]> Co-authored-by: Valentin Kovalenko <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://jira.mongodb.org/browse/HIBERNATE-5
The following static analysis gradle plugins are integrated: