Use Gradle properties #93
Conversation
…l variant, we now provide some methods that are able to compute the former variant fields. We are going to use Gradle properties to fully work in the lazy mode. The concept of variant that is computed after project evaluation is not really compatible with this approach.
…ation of all tasks to follow Gradle new guidelines.
…cOverrides and nodeModulesOutputFilter.
|
I realized that I forgot to replace usages of |
…ontain null values. If they are not defined, they are empty and thus contain nothing and not null.
…es that I don't know well and I am not sure I can migrate them correctly to Kotlin.
…. Add a property to run them using all the Gradle supported versions. The CI still run tests using all Gradle versions.
…o be no longer used (artifactory publication). Simply use the Gradle plugin publish plugin to publish it to Gradle portal.
… model accessor for the node extension.
Change the semver configuration to change to the next major version.
| val download = project.objects.property<Boolean>().convention(false) | ||
|
|
||
| init { | ||
| distBaseUrl.set("https://nodejs.org/dist") |
There was a problem hiding this comment.
Does this interfere with distBaseUrl = null, the way we disabled automatic adding of repositories (for those with corporate environments)?
There was a problem hiding this comment.
Yes, I has to do with unsetting this property.
I learned a few important details regarding the Property API:
- By default, a property is not defined (its value is null). If you want to unset it, you have to set null value.
- Using a convention is not the same thing as setting an initial value. The convention value is a value that will be used if the value is not defined when retrieving it. That means that if you set a value and unset it, it will return the convention value.
- When the property is not defined (its value is null), calling
.map()will do nothing. It took me a lot of time to understand that one test was broken because of that. It was about an@Outputannotation returning aProvider(it sounds like it is recommended to do that). I wanted to map an optional configuration value and define the output according to is value. When the property was not set, theProviderwas considered as empty and the mapping function that handled thenullcase was not called. I had to break the provider chain at this place. I did not find any alternative solution.
To come back to your question, if I use a convention, the property not set but you just have a default value. Unsetting the value won't change anything (since it is already not set). To get it unsettable, we cannot use convention but we have to initially set the value: this will enable the value to be unsettable later.
|
Thanks @deepy for your feedback! Sorry, it will probably take you some time to review that but getting your feedback is very interesting to build a better product. I selected the issues that we should fix before the 3.x release. After, we will no longer have the opportunity to break backward compatibility. All the issues are in the 3.0 milestone. If you think about other issues that should be fixed before the release, don't hesitate to add them to this milestone. Otherwise, I think that we are close to the release of this new major version! |
As discussed in #39, I rewrote the
NodeExtensionand all tasks to use Gradle properties and providers.The code is a little less readable than before but it works. The Kotlin DSL usage is also a little less beautiful than before because we need to use
.set()for all properties.I had to remove the concept of variant that was not really compliant with the concept of lazyness since it was computed after project evaluation. It remains a few usages of
project.afterEvaluate()that I was not able to remove.I also found a better way to handle
execOverridesandnodeModulesOutputFilter. They now use GradleActions as recommended by Gradle because its neutral and the Groovy and Kotlin DSL are able to map Groovy closures and Kotlin lambdas to Actions.What do you think about these changes @deepy and @jfbibeau?