Repository navigation
Fail the build on compiler warnings and check the release bundle in CI - #849
Merged
Merged
Conversation
- Require org.jspecify (static) in module-info. graphql-java's @nullable annotations were unreadable from the module, which Kotlin 2.4 turns into an error (KT-80247). This is what broke the Kotlin 2.4 update. - Hide introspection with BlockedFields instead of the deprecated NoIntrospectionGraphqlFieldVisibility, which does the same thing. Its suggested replacement, Introspection.enabledJvmWide, is JVM-wide. - Drop @ExperimentalCoroutinesApi from SchemaParserOptions.build() and defaultOptions(). Nothing in them is experimental anymore, and Kotlin callers no longer need to opt in. - Suppress the deprecation on the legacy directive argument values and the safe call on the schema, which is null in hand-built environments. - Use Kotlin class literals and drop a redundant cast; fix test warnings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pass -Werror to kotlinc and failOnWarning to javac, so warnings show up in mvn verify instead of later in Sonar or a dependency update. javac now uses release 11, which also drops its "location of system modules" warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…quests - graphql-java-25: builds and tests against graphql-java 25.1, the oldest version this jar supports, e.g. to catch APIs that only exist in 26 like Directives.BUILT_IN_DIRECTIVES_MAP (6f78f35). - release-bundle: builds the Maven Central bundle with the release profile and Maven 3.9, like the release workflow, without uploading it. It fails if the bundle has files Central rejects (e.g. maven-metadata-local.xml under Maven 3.10) or is missing the pom, jar, sources or javadoc jar. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nothing covered introspectionEnabled(false), which now uses BlockedFields directly. The same test passes on master. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 12, 2026
It pins the same Maven as the release workflow, so it can't catch the Maven 3.10 problem it was added for. What's left (javadoc errors, a missing sources or javadoc jar) is rare and fails the release before anything is published or tagged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SchemaParserOptions.build() isn't experimental anymore, so the tests don't need to opt in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
graphql-java always sets the schema on the environment, so the safe call on it was only there for this test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
2 tasks done
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Unblocks #809
Checklist
Description
A few mistakes only showed up after they were merged. Sonar flagged deprecated calls in #834, the Kotlin 2.4 update (#809) broke on a warning we've had for a while, and 6f78f35 had to fix a graphql-java 26 only API we started using. This makes
mvn verifyand the PR checks catch these instead.Compiler warnings fail the build now (
-Werrorfor kotlinc,failOnWarningfor javac). javac also uses<release>11</release>instead of source/target.To get there I fixed all 42 existing warnings (23 main, 19 test). Most are mechanical: Kotlin class literals instead of
java.lang.Object/java.util.Map, a redundant cast, casts in tests. The ones worth a look:module-infonow hasrequires static org.jspecify. graphql-java's@Nullableannotations weren't readable from our module, which Kotlin 2.3 warns about ("Type annotation class 'Nullable' of the inferred type is inaccessible", KT-80247) and 2.4 turns into an error. That's what breaks Update kotlin #809. This branch builds and passes with Kotlin 2.4.21 and coroutines 1.11.0, the versions in Update kotlin #809.introspectionEnabled(false)usesBlockedFields.newBlock().addPattern("__.*")instead of the deprecatedNoIntrospectionGraphqlFieldVisibility, which is exactly what that class does internally. The replacement graphql-java suggests,Introspection.enabledJvmWide, is JVM-wide so it doesn't work for a per-schema option. Nothing tested this option so I addedIntrospectionTest, which passes on master too.SchemaParserOptions.build()anddefaultOptions()aren't marked@ExperimentalCoroutinesApianymore. Nothing in them is experimental, and Kotlin callers had to opt in just to build options. Removing the marker doesn't break source or binary compatibility. The tests that opted in for it don't anymore.@Suppress(it already has a TODO to go away).MethodFieldResolverdoesn't use a safe call onenvironment.graphQLSchemaanymore. graphql-java marks it non-null and always sets it, it was only null in the environmentsMethodFieldResolverDataFetcherTestbuilds by hand, so those get a schema now. Check for scalar arguments when building the schema instead of on every request #859 would drop the lookup altogether.A new PR job,
graphql-java-25, runsverifyagainst graphql-java 25.1, the oldest version we support. It would've caught what 6f78f35 fixed. It isn't a required check yet. Thebuild (...)job names didn't change so branch protection still matches. If it looks fine we can make it required.mvn verifypasses on graphql-java 26.1 and 25.1.Heads up for the open PRs: nothing conflicts, but once this is merged #842 has 2 warnings (
Object::class.javaandjava.util.Map::class.javainSchemaClassScanner) and #843 has 12 (deprecated legacy directive calls inDirectiveTest). #838 is fine.🤖 Generated with Claude Code