Skip to content
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

Change execOverrides to Action<ExecSpec> #73

Closed
wants to merge 8 commits into from
Closed

Conversation

deepy
Copy link
Member

@deepy deepy commented Feb 25, 2020

This hopefully fixes #72

bsautel added 5 commits April 4, 2020 10:46
- Stop downloading Node.js for each integration test project. We only test it once with a specified version and with the default version. Same thing for npm and yarn.
  - This means we rely on the tools installed globally on the system (this is a new development prerequisite).
  - Add new tests to ensure that we correctly use those globally installed tools.
- Remove some tests that are duplicates of other ones (mostly for npm rule integration tests what test npm related features already tested elsewhere).
…, npm, npx and yarn tasks.

Add an integration test using the Kotlin DSL to ensure it is correctly usable.
…execOverrides option in the Groovy DSL as expected but it is no longer the case with the Kotlin DSL.
@bsautel
Copy link
Contributor

bsautel commented Apr 5, 2020

@deepy, I merged your execoverrides branch with the branch I am currently working on in order to get faster tests execution. This also provides the Kotlin DSL test I added.

I thought I already tested the execOverrides option in integration tests but it was not the case, so I added some tests. Without your fix, it indeed does not work using the Groovy DSL. I can confirm you that this fix solves the issue for the Groovy DSL. But now we have the Kotlin DSL test, it appears that your change breaks it. It is the reason why the build of this branch is broken.

Also, for some reason (which may be related to the Groovy DSL magic), your fix does not work with Gradle 5.0. I don't know exactly as of which version it works, but it works with Gradle 5.6.4. I could workaround this by skipping this test in Gradle 5.

…ides property. It still does not work with Gradle < 5.6 (like the Groovy DSL).
@bsautel
Copy link
Contributor

bsautel commented Apr 5, 2020

I found a way to improve the compatibility with the Kotlin DSL but it still does not work with Gradle 5.0 (and probably other versions before Gradle 5.6.4).

@bsautel
Copy link
Contributor

bsautel commented Apr 8, 2020

I close this pull request, I integrated it to #90.

@bsautel bsautel closed this Apr 8, 2020
@deepy
Copy link
Member Author

deepy commented Apr 8, 2020

With Gradle 6 released I'm not sure we want to put too much effort into supporting 5.0.
5.6 yes, for sure, but 5.0 has questionable value.

@bsautel
Copy link
Contributor

bsautel commented Apr 9, 2020

Yes. I read that Spring Boot 2.3 which will be released soon will support only Gradle 5.6 and 6. We can probably do the same thing.

@bsautel bsautel deleted the execoverrides branch April 19, 2020 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Groovy DSL issue with execOverrides and nodeModulesOutputFilter properties
2 participants