Skip to content

build: use JSpecify @Nullable instead of JSR305#444

Merged
vbarua merged 2 commits into
substrait-io:mainfrom
nielspardon:par-checker
Jul 23, 2025
Merged

build: use JSpecify @Nullable instead of JSR305#444
vbarua merged 2 commits into
substrait-io:mainfrom
nielspardon:par-checker

Conversation

@nielspardon
Copy link
Copy Markdown
Member

JSR305 has been dormant for years and was never finalized. It would be better to use alternative approaches like the checkerframework which is e.g. what Apache Calcite is doing.

@nielspardon
Copy link
Copy Markdown
Member Author

looks like a transient build error due to our faulty Java version config. should be fixed by #443 or #433

@nielspardon
Copy link
Copy Markdown
Member Author

rebased on latest main and the build issue is gone

@bestbeforetoday
Copy link
Copy Markdown
Member

Based on a little searching, I am not sure that checkerframework is something we should be buying in to:

JSpecify seems to have a lot of industry backing (Google, Jetbrains, Meta, Microsoft, Oracle, PMD, ...) so maybe that is a better direction?

@nielspardon
Copy link
Copy Markdown
Member Author

Based on a little searching, I am not sure that checkerframework is something we should be buying in to:

I don't mind which alternative we use we just shouldn't use JSR305 in my opinion.

Signed-off-by: Niels Pardon <par@zurich.ibm.com>
Signed-off-by: Niels Pardon <par@zurich.ibm.com>
@nielspardon
Copy link
Copy Markdown
Member Author

I rebased on current main and pushed a commit switching to JSpecify

@nielspardon nielspardon changed the title chore: use checkerframework @Nullable instead of JSR305 chore: use JSpecify @Nullable instead of JSR305 Jul 23, 2025
Copy link
Copy Markdown
Member

@bestbeforetoday bestbeforetoday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me but really needs the blessing of @vbarua on the preferred nullability annotation library.

@vbarua
Copy link
Copy Markdown
Member

vbarua commented Jul 23, 2025

I actually don't have strong preferences here. As @/bestbeforetoday points out, JSpecify seems to have broad industry support so I'm happy to switch to it.

Copy link
Copy Markdown
Member

@vbarua vbarua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vbarua vbarua changed the title chore: use JSpecify @Nullable instead of JSR305 build: use JSpecify @Nullable instead of JSR305 Jul 23, 2025
@vbarua vbarua merged commit 30c3342 into substrait-io:main Jul 23, 2025
12 checks passed
@nielspardon nielspardon deleted the par-checker branch August 5, 2025 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants