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

[#1291] build.gradle for 'picocli-shell-jline3' submodule is changed: Gradle configuration is switched (from 'implementation' to 'api') for 'org.jline:jline' dependency #1292

Merged
merged 1 commit into from
Jan 3, 2021

Conversation

dejan2609
Copy link
Contributor

@dejan2609 dejan2609 commented Jan 2, 2021

note: solves broken build issues for any Maven project that uses 'picocli-shell-jline3' version 4.6.0

related issue: #1291 Cannot compile with picocli-shell-jline3 dependency using Maven

…anged: Gradle configuration is switched (from 'implementation' to 'api') for 'org.jline:jline' dependency

note: solves broken build issues for any Maven project that uses 'picocli-shell-jline3' version 4.6.0
@codecov-io
Copy link

codecov-io commented Jan 2, 2021

Codecov Report

Merging #1292 (71e9348) into master (572e05f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1292   +/-   ##
=========================================
  Coverage     93.78%   93.78%           
  Complexity      469      469           
=========================================
  Files             2        2           
  Lines          6948     6948           
  Branches       1864     1864           
=========================================
  Hits           6516     6516           
  Misses          146      146           
  Partials        286      286           

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 572e05f...71e9348. Read the comment docs.

@remal
Copy link

remal commented Jan 2, 2021

I suppose the same change should be applied to all other modules, as we have the same issue for picocli-spring-boot-starter module: #1294

@@ -13,7 +13,7 @@ targetCompatibility = 1.8

dependencies {
implementation rootProject
Copy link

Choose a reason for hiding this comment

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

Shouldn't we change this to api too?

Copy link
Contributor Author

@dejan2609 dejan2609 Jan 2, 2021

Choose a reason for hiding this comment

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

Thanx for a review @remal ! I will push another commit for picocli-spring-boot-starter tomorrow (and hopefully all these commits will be squashed into one if/when this gets merged into master).

As for a rootProject... To be honest: this is the first that I see this Gradle directive used as a dependency (I presume that it references code/dependency from root/src/ (that somehow didn't find its way into some separate submodule).

⏩ Going further: if we compare transitive dependencies for picocli-shell-jline3 recent versions (4.5.2 vs. 4.6.0):

we will found out that Maven scope changed from compile to runtime for these three dependencies:

  1. info.picocli:picocli (root project, mentioned above)
  2. org.jline:jline (https://github.com/jline/jline3/blob/master/jline/pom.xml)
  3. org.jline:jline-console (https://github.com/jline/jline3/blob/master/console/pom.xml)

It seems that Gradle configuration for all of those three dependencies should be changed from implementation to api in order to re-introduce Maven scope compile (when those dependencies are used transitively, that is).

I will have to sleep on this 😴

@remkop remkop added this to the 4.6.1 milestone Jan 3, 2021
@remkop remkop merged commit 24c3fe5 into remkop:master Jan 3, 2021
@remkop
Copy link
Owner

remkop commented Jan 3, 2021

Thank you both for the contribution!
This was a great help in understanding what needed to be done.
Picocli 4.6.1 has been released with these changes.

@dejan2609
Copy link
Contributor Author

No problem @remkop ! I see that you were super-fast and that changes are landed into master: https://github.com/remkop/picocli/commits/v4.6.1 😃

@dejan2609 dejan2609 deleted the develop branch January 3, 2021 07:59
@remkop
Copy link
Owner

remkop commented Jan 3, 2021

@dejan2609 Thank you for your help! 👍🙏

MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this pull request Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants