Skip to content

Fix gradle module metadata dependencies #1490

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

Merged

Conversation

leonard84
Copy link
Member

Prior to this commit, the gradle module metadata didn't match the generated pom,
it included more dependencies as Gradle has no concept of provided/optional dependencies.
To fix this we consequently use compileOnly for dependencies that are optional.
This will omit these dependencies from both the pom and gmm.

fixes #1469

@leonard84 leonard84 self-assigned this Jun 29, 2022
@leonard84 leonard84 force-pushed the fix_gradle_module_dependencies branch from 93b073b to 0f03ebe Compare June 29, 2022 15:39
@leonard84 leonard84 requested a review from marcphilipp June 29, 2022 16:06
@leonard84 leonard84 added this to the 2.2 milestone Jun 29, 2022
@leonard84 leonard84 force-pushed the fix_gradle_module_dependencies branch from 0f03ebe to ade3add Compare June 30, 2022 08:02
Comment on lines -9 to +10
implementation "com.google.inject:guice:3.0", provided
compileOnly "com.google.inject:guice:3.0"
testImplementation "com.google.inject:guice:3.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these (and below for the other specific modules) be normal implementation dependencies? Consumers could still use different versions by using Gradle's dependency management capabilities.

Copy link
Member Author

@leonard84 leonard84 Jul 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They had been marked as provided in Maven, which basically makes them non-transitive. This PR just aligns the behavior between Gradle and Maven. And you shouldn't get your DI framework from the test framework as transitive dependency, it should be a compile/implementation dependency.

Prior to this commit, the gradle module metadata didn't match the generated pom,
it included more dependencies as Gradle has no concept of provided/optional dependencies.
To fix this we consequently use `compileOnly` for dependencies that are optional.
This will omit these dependencies from both the pom and gmm.

fixes spockframework#1469
@leonard84 leonard84 force-pushed the fix_gradle_module_dependencies branch from ade3add to 2bd6832 Compare July 8, 2022 13:28
@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #1490 (b8cbbac) into master (b9e05d5) will not change coverage.
The diff coverage is n/a.

❗ Current head b8cbbac differs from pull request most recent head 37210a5. Consider uploading reports for the commit 37210a5 to get more accurate results

@@            Coverage Diff            @@
##             master    #1490   +/-   ##
=========================================
  Coverage     79.73%   79.73%           
- Complexity     4023     4025    +2     
=========================================
  Files           411      411           
  Lines         12715    12715           
  Branches       1650     1649    -1     
=========================================
  Hits          10138    10138           
  Misses         1982     1982           
  Partials        595      595           

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 b9e05d5...37210a5. Read the comment docs.

@leonard84 leonard84 merged commit 30819ae into spockframework:master Jul 13, 2022
@leonard84 leonard84 deleted the fix_gradle_module_dependencies branch July 13, 2022 17:56
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.

Depdency resolution must be consistent between maven and gradle
2 participants