convert FilePermissionsTask.groovy to .java #34674
Conversation
|
Pinging @elastic/es-core-infra |
buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java
Outdated
Show resolved
Hide resolved
|
@elasticmachine test this please |
|
@vboulaye looks like CI uncovered checkstyle rules. |
1 similar comment
|
@vboulaye looks like CI uncovered checkstyle rules. |
|
@elasticmachine test this please |
buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java
Outdated
Show resolved
Hide resolved
buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java
Outdated
Show resolved
Hide resolved
buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java
Outdated
Show resolved
Hide resolved
buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java
Outdated
Show resolved
Hide resolved
buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java
Show resolved
Hide resolved
buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java
Outdated
Show resolved
Hide resolved
|
@elasticmachine test this please |
| return outputMarker; | ||
| } | ||
|
|
||
| public void setOutputMarker(File outputMarker) { |
There was a problem hiding this comment.
We don't need this to be configurable, it can be considered an implementation detail.
Sorry, I missed this in my earlier review.
There was a problem hiding this comment.
I dropped it, and found out that I needed to create the missing parent folders (my previous test used an output file at the root of the TemporaryFolder). I added a .mkdirs() in the Task.
| SourceSetContainer sourceSets = getProject().getConvention().getPlugin(JavaPluginConvention.class).getSourceSets(); | ||
| return getProject().files(sourceSets.stream() | ||
| .map(sourceSet -> sourceSet.getAllSource().matching(filesFilter)) | ||
| .toArray()); |
There was a problem hiding this comment.
FileTree extends FileCollection, so neither getProject().files( nor .toArray() are necessary.
There was a problem hiding this comment.
sorry, I do not know the API that well...
I tried to fix this but I am not sure how to handle the case when no source matches? (I do not like the .orElse(null))
There was a problem hiding this comment.
The .orElse(null) worked in the unit tests but wouldn't have worked in an integration test as the method is annotated as input and thus can't return null. I took the liberty to push a fix for this.
|
@elasticmachine ok to test |
related to #34459, convert the FilePermissionsTask.groovy to java