Skip to content
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

Fix checkstyle version drift and API change #88283

Merged
merged 3 commits into from
Jul 5, 2022

Conversation

pugnascotia
Copy link
Contributor

It turns out we have the Checkstyle version in 2 places:

  • build-tools-internal/version.properties
  • gradle/build.versions.toml

This concealed that there was an API change between 10.1 and 10.3, and we needed to update one of our custom checks.

@pugnascotia pugnascotia requested a review from breskeby July 5, 2022 15:49
@elasticmachine elasticmachine added the Team:Delivery Meta label for Delivery team label Jul 5, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@rjernst
Copy link
Member

rjernst commented Jul 5, 2022

Why didn’t the build fail with this conflict? Can we fail going forward in this situation so that we don’t have this jarhell again?

Copy link
Contributor

@breskeby breskeby left a comment

Choose a reason for hiding this comment

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

lgtm

@pugnascotia
Copy link
Contributor Author

We don't apply the custom Checkstyle in the CLI build, only in the IDE. We would have caught this if we didn't duplicate the Checkstyle version declaration. @breskeby said he'd look into consolidating them.

…nal/checkstyle/MissingJavadocTypeCheck.java

Co-authored-by: Rene Groeschke <[email protected]>
@pugnascotia pugnascotia merged commit a57e645 into elastic:master Jul 5, 2022
@pugnascotia pugnascotia deleted the fix-checkstyle-version branch July 5, 2022 20:37
pugnascotia added a commit to pugnascotia/elasticsearch that referenced this pull request Jul 5, 2022
Fix checkstyle version drift and API change.
pugnascotia added a commit to pugnascotia/elasticsearch that referenced this pull request Jul 5, 2022
Fix checkstyle version drift and API change.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
7.17
8.3

elasticsearchmachine pushed a commit that referenced this pull request Jul 5, 2022
Fix checkstyle version drift and API change.
elasticsearchmachine pushed a commit that referenced this pull request Jul 5, 2022
Fix checkstyle version drift and API change.
breskeby added a commit that referenced this pull request Jul 15, 2022
We only rely on the checkstyle version in the buildLibs.toml gradle version catalogue with this change.
Also added some hints for gradle best practices.

This is an aftermath of #88283
breskeby added a commit to breskeby/elasticsearch that referenced this pull request Jul 18, 2022
We only rely on the checkstyle version in the buildLibs.toml gradle version catalogue with this change.
Also added some hints for gradle best practices.

This is an aftermath of elastic#88283
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v7.17.6 v8.3.2 v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants