-
Notifications
You must be signed in to change notification settings - Fork 886
CASSJAVA-102: Fix revapi surious complaints about optional dependencies #2042
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: 4.x
Are you sure you want to change the base?
Conversation
Example error: [ERROR] Failed to execute goal org.revapi:revapi-maven-plugin:0.15.1:check (default) on project java-driver-core: The following API problems caused the build to fail: [ERROR] java.field.enumConstantOrderChanged: field com.datastax.oss.driver.api.core.config.DefaultDriverOption.ADDRESS_TRANSLATOR_CLASS: The enum constant was defined on position 61 but is now on 62. The user code can break if it relies on the return value of the "ordinal()" method. https://revapi.org/revapi-java/differences.html#java.field.enumConstantOrderChanged ...
This reverts commit b87a112.
…arargParameter is enabled [ERROR] Failed to execute goal org.revapi:revapi-maven-plugin:0.15.1:check (default) on project java-driver-query-builder: The following API problems caused the build to fail: [ERROR] java.method.varargOverloadsOnlyDifferInVarargParameter: method com.datastax.dse.driver.api.querybuilder.schema.AlterDseTableDropColumnEnd com.datastax.dse.driver.api.querybuilder.schema.AlterDseTableDropColumn::dropColumns(com.datastax.oss.driver.api.core.CqlIdentifier[]): Method only differs in the vararg type from some of its overloads: [method com.datastax.dse.driver.api.querybuilder.schema.AlterDseTableDropColumnEnd com.datastax.dse.driver.api.querybuilder.schema.AlterDseTableDropColumn::dropColumns(java.lang.String[])]. https://revapi.org/revapi-java/differences.html#java.method.varargOverloadsOnlyDifferInVarargParameter [ERROR] java.method.varargOverloadsOnlyDifferInVarargParameter: method com.datastax.dse.driver.api.querybuilder.schema.AlterDseTableDropColumnEnd com.datastax.dse.driver.api.querybuilder.schema.AlterDseTableDropColumn::dropColumns(java.lang.String[]): Method only differs in the vararg type from some of its overloads: [method com.datastax.dse.driver.api.querybuilder.schema.AlterDseTableDropColumnEnd com.datastax.dse.driver.api.querybuilder.schema.AlterDseTableDropColumn::dropColumns(com.datastax.oss.driver.api.core.CqlIdentifier[])]. https://revapi.org/revapi-java/differences.html#java.method.varargOverloadsOnlyDifferInVarargParameter [ERROR] java.method.varargOverloadsOnlyDifferInVarargParameter: method com.datastax.oss.driver.api.querybuilder.relation.MultiColumnRelationBuilder<SelfT> com.datastax.oss.driver.api.querybuilder.relation.OngoingWhereClause<SelfT extends com.datastax.oss.driver.api.querybuilder.relation.OngoingWhereClause<SelfT>>::whereColumns(com.datastax.oss.driver.api.core.CqlIdentifier[]): Method only differs in the vararg type from some of its overloads: [method com.datastax.oss.driver.api.querybuilder.relation.MultiColumnRelationBuilder<SelfT> com.datastax.oss.driver.api.querybuilder.relation.OngoingWhereClause<SelfT extends com.datastax.oss.driver.api.querybuilder.relation.OngoingWhereClause<SelfT>>::whereColumns(java.lang.String[])]. https://revapi.org/revapi-java/differences.html#java.method.varargOverloadsOnlyDifferInVarargParameter
I can confirm the following:
At this point my only hesitation involves the increase in the required Maven version to build with this change. I thought we either (a) enforced that somewhere in the pom files or (b) specified it in documentation somewhere... but at the moment I can't find it in either. |
@absurdfarce Could we address your hesitation by merge + deploy images in apache/cassandra-builds#117, then let CI here go green, and merge? |
I don't think that covers everything @aratno. I'm worried about somebody trying to download the source and build locally... without either some guidance from us or enforcement via Maven they won't have any idea what version to use. I thought we specified this somewhere in our docs... I have a vague recollection of having to update this information when we tweaked Maven versions last time. But it's entirely possible I'm just making that up. I'll do some more checking on my end and post an update here. I don't want to hold up getting this in on a minor point like this; if we don't get to an answer pretty quickly I'm going to suggest we just add recommendations to the docs about which Maven version to use and move on. |
I'll try out maven-enforcer-plugin for that: https://maven.apache.org/enforcer/maven-enforcer-plugin/usage.html If that doesn't work nicely, will document in README. |
Just pushed a configuration for maven-enforcer-plugin that worked for me with 3.8.8 locally, tested a few combinations of version rules with |
@@ -27,7 +27,7 @@ def initializeEnvironment() { | |||
env.GITHUB_BRANCH_URL = "${GITHUB_PROJECT_URL}/tree/${env.BRANCH_NAME}" | |||
env.GITHUB_COMMIT_URL = "${GITHUB_PROJECT_URL}/commit/${env.GIT_COMMIT}" | |||
|
|||
env.MAVEN_HOME = "${env.HOME}/.mvn/apache-maven-3.6.3" | |||
env.MAVEN_HOME = "${env.HOME}/.mvn/apache-maven-3.8.8" |
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 won't work out of the gate; I have to update the AMI def for the runner that executes this logic to include a new Jenkins version. This can probably stay as 3.8.8 is the current 3.8.x release as of this writing... I'm just saying it's not enough by itself.
I'll take care of adding the new version to the runner.
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.
Thanks for taking care of that - the definition of the AMI isn't in a place I can modify it (DS-internal)
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.
PR to make this change is now up
Enforcer plugin did what it says on the tin... so I'm satisfied here. |
Dependent PR has been approved, waiting for Mick to deploy the new images: apache/cassandra-builds#117 Then we'll run CI here again and confirm it's working, then merge. |
https://issues.apache.org/jira/browse/CASSJAVA-102