refactor: convert NoticeTask to java #34769
Conversation
| */ | ||
| public class NoticeTask extends DefaultTask { | ||
|
|
||
| private static final Charset UTF8 = Charset.forName("UTF-8"); |
There was a problem hiding this comment.
no need for a constant here, you can use StandardCharsets.UTF_8.
| public class NoticeTask extends DefaultTask { | ||
|
|
||
| private static final Charset UTF8 = Charset.forName("UTF-8"); | ||
| @InputFile |
There was a problem hiding this comment.
For java, there needs to be setters/getters, as the annotation will not automatically create them as it does in groovy. The annotations then go on the getters/settings.
| /** | ||
| * Add notices from the specified directory. | ||
| */ | ||
| public void setLicensesDir(File licensesDir) { |
There was a problem hiding this comment.
The method was not named as a setter in groovy so this could be DSL-like. ie, usage looks like (notice the lack of equals sign):
noticeTask {
licensesDir 'foo'
}
There was a problem hiding this comment.
Note that this is different than setting a single property as it adds the inputs to the list.
| String name = file.getName().substring(0, file.getName().length() - "-NOTICE.txt".length()); | ||
| if (seen.containsKey(name)) { | ||
| File prevFile = seen.get(name); | ||
| if (!read(prevFile).equals(read(file))) { |
There was a problem hiding this comment.
We use == False throughout the codebase instead of ! because the former is easier to spot visually.
|
Pinging @elastic/es-core-infra |
|
@baptistemesta thank you for your PR! Are you still interested to get it in? If yes, it would be great if you can address the review comments; then we can proceed. |
|
@baptistemesta I'm going to close this due to lack of response, please re-open it if you plan to work on it. |
6 participants
Relates to #34459
I was not sure about tests. Would it be interesting here to test taskInput/output?