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

Make the plugin compatible with Gradle's configuration cache #157

Merged
merged 8 commits into from
Jul 4, 2020

Conversation

eskatos
Copy link
Contributor

@eskatos eskatos commented Jun 21, 2020

See #156

Note that the configuration cache will be introduced in Gradle 6.6.
This pull-request currently add integration testing with 6.6-milestone-2.
6.6 RC1 is around the corner.

This PR doesn't change the supported Gradle versions and let the plugin still work with Gradle >= 2.0.

eskatos added 6 commits June 21, 2020 13:50
Signed-off-by: Paul Merlin <[email protected]>
Signed-off-by: Paul Merlin <[email protected]>
Signed-off-by: Paul Merlin <[email protected]>
by removing any use of `Project` at execution time

Signed-off-by: Paul Merlin <[email protected]>
by making the ProjectBuilder build offline before applying the plugin
now that the offline start parameter is read earlier

Signed-off-by: Paul Merlin <[email protected]>
@codecov
Copy link

codecov bot commented Jun 21, 2020

Codecov Report

Merging #157 into master will decrease coverage by 0.62%.
The diff coverage is 72.30%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #157      +/-   ##
============================================
- Coverage     80.10%   79.48%   -0.63%     
- Complexity      233      244      +11     
============================================
  Files            13       15       +2     
  Lines           769      809      +40     
  Branches        117      121       +4     
============================================
+ Hits            616      643      +27     
- Misses           92      102      +10     
- Partials         61       64       +3     
Impacted Files Coverage Δ Complexity Δ
...adle/tasks/download/internal/ProjectApiHelper.java 50.00% <50.00%> (ø) 2.00 <2.00> (?)
...dercouch/gradle/tasks/download/DownloadAction.java 76.28% <63.63%> (+0.24%) 134.00 <2.00> (+1.00)
...tasks/download/internal/ProgressLoggerWrapper.java 74.41% <76.92%> (-1.34%) 10.00 <4.00> (+4.00) ⬇️
.../de/undercouch/gradle/tasks/download/Download.java 89.13% <100.00%> (-3.18%) 43.00 <0.00> (ø)
...couch/gradle/tasks/download/DownloadExtension.java 84.61% <100.00%> (+1.28%) 2.00 <1.00> (ø)
...undercouch/gradle/tasks/download/VerifyAction.java 90.47% <100.00%> (ø) 16.00 <0.00> (ø)
...ownload/internal/ProgressLoggerFactoryWrapper.java 100.00% <100.00%> (ø) 4.00 <4.00> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 687ef29...e42fae1. Read the comment docs.

eskatos added 2 commits June 21, 2020 17:16
lowest supported Gradle version for the used APIs

Signed-off-by: Paul Merlin <[email protected]>
instead of a nightly

Signed-off-by: Paul Merlin <[email protected]>
@eskatos eskatos marked this pull request as ready for review June 23, 2020 13:02
@eskatos
Copy link
Contributor Author

eskatos commented Jun 24, 2020

Hi @michel-kraemer,

I'm sure you were already notified of this PR but I have some specific questions.

There are two ways of doing integration testing of Gradle versions in the plugin's build. One in gradle/integrationTest.gradle. The other in FunctionalDownloadTest. This PR currently changes gradle/integrationTest.gradle only. Are my changes to gradle/integrationTest.gradle ok or would you prefer another approach?

This PR uses 6.6-milestone-2 for testing, it's not final but it's a released version so the artifacts will never disappear. Is it a blocker to you for merging this?

I also would like some guidance on the codecov checks on this PR. I assume the coverage ratio going down a bit is because of the added branches per Gradle version and that code coverage is only collected for one version. Does that sounds right to you? Do you want me to rework the code and add some unit test coverage to gain those precious percents back?

Now for some context, I work with large builds that make use of your very useful plugin. These builds are suffering from long Gradle configuration time and being able to use the configuration cache is a life saver for their "users". Thank you for considering that PR.

@michel-kraemer michel-kraemer merged commit e42fae1 into michel-kraemer:master Jul 4, 2020
@michel-kraemer
Copy link
Owner

@eskatos Hey! Sorry for my late response. I was really busy last week.

Thank you so much for the hard and thorough work. This looks really impressive. I did some minor edits (mostly in terms of code style but nothing functional).

Don't worry about the different integration tests or the code coverage. Everything looks OK. Also, I'm fine with the milestone. We can replace it later.

These builds are suffering from long Gradle configuration time and being able to use the configuration cache is a life saver for their "users". Thank you for considering that PR.

Sure thing. I'm happy to hear that my work is useful for you!

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.

2 participants