Moved lucene.java.tests-and-randomization.gradle and more scripts to java #14897
Conversation
github-project-automation
bot
moved this to Open
in OpenSearch Lucene & Core Performance Tracking
|
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR. |
dweiss
added
the
skip-changelog
…passing some ext properties to an extension.
dweiss
changed the title
|
I think this is already large enough and fairly self-isolated. I'd appreciate if you could take a look, @breskeby, although I'm sure it's not light reading. |
2 similar comments
|
I think this is already large enough and fairly self-isolated. I'd appreciate if you could take a look, @breskeby, although I'm sure it's not light reading. |
|
I think this is already large enough and fairly self-isolated. I'd appreciate if you could take a look, @breskeby, although I'm sure it's not light reading. |
build-tools/build-infra/src/main/groovy/lucene.documentation.render-javadoc.gradle
Outdated
Show resolved
Hide resolved
| * resources are located under the top-level {@code gradle/} folder. | ||
| */ | ||
| protected Path gradlePluginResource(Project project, String relativePath) { | ||
| return project |
There was a problem hiding this comment.
Instead of project.rootProject.layout.projectDir you can use project.layout. getSettingsDirectory()
This avoids crossreferencing root project here.
There was a problem hiding this comment.
I do remember your comment from before but getSettingsDirectory pops up as "unstable" API though... is it ok?
There was a problem hiding this comment.
I've changed it to getSettingsDirectory, even though it's marked as unstable API (incubating).https://docs.gradle.org/current/javadoc/org/gradle/api/file/ProjectLayout.html#getSettingsDirectory()
| jvmMetadata.getJavaHome()); | ||
| }; | ||
|
|
||
| project |
There was a problem hiding this comment.
Not sure I would create a task just for a logger message
There was a problem hiding this comment.
The reason for it is that I only want to get it printed once. I couldn't figure out any other clean way of doing it. Also, if you log during configuration time, the log message is stripped of the "context" (task where it comes from) and I think it's nice to be able to track it back to something more concrete. If you know how to implement this in a different way, let me know.
| .withType(Test.class) | ||
| .configureEach( | ||
| task -> { | ||
| task.dependsOn(":altJvmWarning"); |
There was a problem hiding this comment.
Can't you just put the logging here?
There was a problem hiding this comment.
Explained above - it'd print the log message multiple times. Not ideal, I think.
There was a problem hiding this comment.
Ultimately the way for those loggings would be to use the problems api noawadays but again that's an overload to just introduce this for this warning. I have used that while working on the license checks pr. I'd suggest to leave it as is for now and we can look into leverage the problems api at another time Dir more placing to have a central place for these warnings
...s/build-infra/src/main/java/org/apache/lucene/gradle/plugins/java/RenderJavadocTaskBase.java
Outdated
Show resolved
Hide resolved
…ugh it's reported as 'unstable API'. We use internal APIs in other places too, we'll worry about it when they change.
|
I am going to merge this one in, but I'd love to see what you have to say with regard to the comments above. If there is a better way to achieve the goal (one-time log message), I'll follow up. |
github-project-automation
bot
moved this from Open
to Merged
in OpenSearch Lucene & Core Performance Tracking
|
Thank you for another of these: I just had to mod these sources on another PR and it is really nice to do it with more confidence, compile-time checks, etc. |
|
I agree. It's more verbose but much easier to write/ modify/ understand. Happy 4th of July! |
Uh oh!
There was an error while loading. Please reload this page.