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

Improve checkstyle performance and readability #54308

Merged
merged 2 commits into from
Mar 27, 2020

Conversation

mark-vieira
Copy link
Contributor

This is a follow up to #53925. In this PR we take the new checkstyle check implementation and embed it in buildSrc rather than creating a new top-level project in the build.

@mark-vieira mark-vieira requested review from rjernst and nik9000 March 26, 2020 23:12
@mark-vieira mark-vieira added the :Delivery/Build Build or test infrastructure label Mar 26, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Build)

nik9000 and others added 2 commits March 26, 2020 16:13
Drop a nasty regex in our checkstyle config that I wrote a long time ago
in favor of a checkstyle extension. This is better because:
* It is faster. It saves a little more than a minute across the entire
  build.
* It is easier to read. Who knew 100 lines of Java would be easier to
  read than a regex, but it is.
* It has tests.
@mark-vieira mark-vieira force-pushed the checkstyle-snippet-check branch from ab1f6da to 961102a Compare March 26, 2020 23:13
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

I'm happy with it. @rjernst, what do you think?

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@nik9000
Copy link
Member

nik9000 commented Mar 27, 2020

Wonderful news!

@mark-vieira mark-vieira merged commit 12580ac into elastic:master Mar 27, 2020
@mark-vieira mark-vieira deleted the checkstyle-snippet-check branch March 27, 2020 15:04
mark-vieira added a commit that referenced this pull request Mar 27, 2020
Drop a nasty regex in our checkstyle config that I wrote a long time ago
in favor of a checkstyle extension. This is better because:
* It is faster. It saves a little more than a minute across the entire
  build.
* It is easier to read. Who knew 100 lines of Java would be easier to
  read than a regex, but it is.
* It has tests.
mark-vieira added a commit that referenced this pull request Mar 27, 2020
Drop a nasty regex in our checkstyle config that I wrote a long time ago
in favor of a checkstyle extension. This is better because:
* It is faster. It saves a little more than a minute across the entire
  build.
* It is easier to read. Who knew 100 lines of Java would be easier to
  read than a regex, but it is.
* It has tests.
mark-vieira added a commit that referenced this pull request Mar 27, 2020
Drop a nasty regex in our checkstyle config that I wrote a long time ago
in favor of a checkstyle extension. This is better because:
* It is faster. It saves a little more than a minute across the entire
  build.
* It is easier to read. Who knew 100 lines of Java would be easier to
  read than a regex, but it is.
* It has tests.
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Mar 30, 2020
* upstream/master: (447 commits)
  Adjust BWC version on node roles being sorted
  Deprecate node local storage setting (elastic#54374)
  Remove Unused BaseNodeRequest (elastic#54323)
  Decouple environment from DiscoveryNode (elastic#54373)
  Ensure that the output of node roles are sorted (elastic#54376)
  Do not stash environment in security (elastic#54372)
  Do not stash environment in machine learning (elastic#54371)
  Clean up how pipeline aggs check for multi-bucket (elastic#54161)
  Remove toString from Painless user tree nodes (elastic#54117)
  Docs: Use splitOnToken instead of custom function (elastic#48408)
  bwc: enable script cache in node stats (elastic#54362)
  bwc: Mute for script cache in node stats (elastic#54359)
  Test to enforce response to invalid data stream names
  Scripting: Use randomValueOtherThan in stats test (elastic#54356)
  Move transport decoding and aggregation to server (elastic#48263)
  Mute ScriptServiceTests.testContextCacheStats (elastic#54352)
  Improve checkstyle performance and readability (elastic#54308)
  Disable Gradle daemon when executing Windows packaging tests (elastic#54316)
  [Docs] Minor fix for SubmitAsyncSearchRequest.keepOnCompletion javadoc (elastic#54325)
  [DOCS] Fix typos in top metrics agg docs (elastic#54299)
  ...

Conflicts:
	server/src/main/java/org/elasticsearch/index/IndexModule.java
	server/src/main/java/org/elasticsearch/index/IndexService.java
	server/src/main/java/org/elasticsearch/indices/IndicesService.java
	server/src/test/java/org/elasticsearch/index/IndexModuleTests.java
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v7.6.3 v7.7.1 v7.8.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants