Add check for feature aware implementations #31081
Merged
Conversation
This file contains 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
This commit adds a check that any class in X-Pack that is a feature aware custom also implements the appropriate mix-in interface in X-Pack. These interfaces provide a default implementation of FeatureAware#getRequiredFeature that returns that x-pack is the required feature. By implementing this interface, this gives a consistent way for X-Pack feature aware customs to return the appopriate required feature and this check enforces that all such feature aware customs return the appropriate required feature.
jasontedor
added
review
:Delivery/Build
|
Pinging @elastic/es-core-infra |
nik9000
approved these changes
Jun 4, 2018
| // filter out non-existent classes directories from empty source sets | ||
| classDirectories = project.files(files).filter { it.exists() } | ||
| } | ||
| description = "Runs FeatureAwareCheck on ${classDirectories.files}." |
There was a problem hiding this comment.
Could you move these out of project.afterEvaluate? I figure those should be as small as possible? I think the description could be "Runs FeatureAwareCheck on main files".
|
|
||
| forbiddenApisMain.enabled = true | ||
| forbiddenApisMain { | ||
| signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')] // does not depend on core, only jdk signatures |
There was a problem hiding this comment.
We have core on the classpath so maybe we can drop this?
There was a problem hiding this comment.
Ah, yeah. That's leftover from when my first implementation was based on strings (names of the classes) rather than now where I took a dependency to be able to refer to the classes directly.
|
run gradle build tests |
|
run packaging sample tests |
|
run gradle build tests |
|
run sample packaging tests |
ywelsch
reviewed
Jun 5, 2018
| @@ -65,6 +65,9 @@ class PrecommitTasks { | |||
| precommitTasks.add(configureLoggerUsage(project)) | |||
| } | |||
|
|
|||
| if (project.path.startsWith(":x-pack:plugin:")) { | |||
There was a problem hiding this comment.
This check does not belong under buildSrc, which should be unaware of the individual projects. Instead, the task should be installed in the build.gradle of the corresponding x-pack directory or could also be packaged as a plugin.
There was a problem hiding this comment.
I pushed commits that do this (in x-pack/plugin/build.gradle, I did not package this as a plugin and I do not intend to at this time).
* elastic/master: [Tests] Muting RatedRequestsTests#testXContentParsingIsNotLenient TEST: Retry synced-flush if ongoing ops on primary (elastic#30978) Fix docs build. Only auto-update license signature if all nodes ready (elastic#30859) Add BlobContainer.writeBlobAtomic() (elastic#30902) Add a doc value format to binary fields. (elastic#30860) Take into account the return value of TcpTransport.readMessageLength(...) in Netty4SizeHeaderFrameDecoder Move caching of the size of a directory to `StoreDirectory`. (elastic#30581) Clarify docs about boolean operator precedence. (elastic#30808) Docs: remove notes on sparsity. (elastic#30905) Fix MatchPhrasePrefixQueryBuilderTests#testPhraseOnFieldWithNoTerms run overflow forecast a 2nd time as regression test for elastic/ml-cpp#110 (elastic#30969) Improve documentation of dynamic mappings. (elastic#30952) Decouple MultiValueMode. (elastic#31075) Docs: Clarify constraints on scripted similarities. (elastic#31076)
* elastic/master: Add cors support to NioHttpServerTransport (elastic#30827) [DOCS] Fixes security example (elastic#31082) Allow terms query in _rollup_search (elastic#30973) Removing erroneous repeat Adapt bwc versions after backporting elastic#30983 to 6.4
jasontedor
added a commit
that referenced
this pull request
Jun 6, 2018
This commit adds a check that any class in X-Pack that is a feature aware custom also implements the appropriate mix-in interface in X-Pack. These interfaces provide a default implementation of FeatureAware#getRequiredFeature that returns that x-pack is the required feature. By implementing this interface, this gives a consistent way for X-Pack feature aware customs to return the appopriate required feature and this check enforces that all such feature aware customs return the appropriate required feature.
jasontedor
added a commit
that referenced
this pull request
Jun 6, 2018
This commit adds a check that any class in X-Pack that is a feature aware custom also implements the appropriate mix-in interface in X-Pack. These interfaces provide a default implementation of FeatureAware#getRequiredFeature that returns that x-pack is the required feature. By implementing this interface, this gives a consistent way for X-Pack feature aware customs to return the appopriate required feature and this check enforces that all such feature aware customs return the appropriate required feature.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This commit adds a check that any class in X-Pack that is a feature aware custom also implements the appropriate mix-in interface in X-Pack. These interfaces provide a default implementation of FeatureAware#getRequiredFeature that returns that x-pack is the required feature. By implementing this interface, this gives a consistent way for X-Pack feature aware customs to return the appopriate required feature and this check enforces that all such feature aware customs return the appropriate required feature.
Relates #31020