-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Upgrade to gradle 8.10 #13700
Upgrade to gradle 8.10 #13700
Conversation
…Example for some reason (but only on gitub).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested a number of different JDKs, 21, 22, and 23-ea, with various different test specific properties and env variables, and all worked successfully. LGTM.
@@ -32,7 +32,7 @@ allprojects { | |||
missingdoclet "org.apache.lucene.tools:missing-doclet" | |||
} | |||
|
|||
ext { | |||
project.ext { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. it's odd that we have to do this, but I see that it's now necessary (why Gradle.. why!? ). This is reason that my previous local attempt to upgrade failed. Good that you have a fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think something within the scope changed and either groovy or gradle looks up a different 'ext' (extension) property now. I didn't bother trying to narrow it down but it is ugly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good. I will report issues in the seldomy executed tasks (like regenerate)
I also tested |
This is already sanity checked when you touch/ alter gradle files, along with other things. Thanks for checking Eclipse, this is a bit manual. |
Fixes #13698.
I can't claim I've tested everything but most things seem to work.