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

Update default gradle configuration #631

Conversation

LouisBoudreau
Copy link
Contributor

Answer to #630 (comment)

I am by no means a Gradle expert. Our use case is an Android application and we use the releaseRuntimeClasspath configuration so I have no experience using the default configuration provided by licensed.

After investigating this I think we should default to only using the runtimeClasspath configuration.
According to the following section of the documentation:

A configuration which has canBeResolved set to false is not meant to be resolved. Such a configuration is there only to declare dependencies. The reason is that depending on the usage (compile classpath, runtime classpath), it can resolve to different graphs. It is an error to try to resolve a configuration which has canBeResolved set to false

Also, looking at JavaPlugin doc, we can see that runtimeClasspath is resolvable.

After making tests I also noticed that no dependencies where listed when running the printDependencies task with only the runtimeOnly configuration making it useless.

Here are some screenshots:

CleanShot 2023-02-24 at 11 03 29@2x

image

I therefore propose to only using runtimeClasspath for the default configuration

@jonabc
Copy link
Contributor

jonabc commented Feb 24, 2023

👋 @LouisBoudreau

I therefore propose to only using runtimeClasspath for the default configuration

After the research and learning I did to get to the place of writing #630 I came to the same general conclusion as what you proposed. I have no prior background with Gradle though and wasn't too confident of that conclusion on my own 😆. 👍 to removing it if it makes sense.

That said, I'd like to leave the logic in place to create the custom licensed configuration overrides. The gradle.configurations input can be set by users making it possible for a non-resolvable configuration to still get used. Using those configurations shouldn't cause errors to get raised if we can work around it.

Do you mind if I cherry-pick 752507c into #630 and close this PR?

@jonabc
Copy link
Contributor

jonabc commented Feb 25, 2023

👍 to removing it if it makes sense.

Having thought about this a bit more, I think I'd prefer to close this PR unmerged and move forward with #630 as-is. This is entirely from the POV that for very widely used tools like package managers, documentation and real-world usage are hardly ever in a 1:1 match. I'm wary of making a breaking change (even one that is easily worked around) based on my small sliver of knowledge about Gradle and feedback/usage from one user.

Just to be clear, I agree with what you said and I think it's probably safe. I'm only looking to reduce risk here and avoid needing to signify breaking changes with a major version bump. I appreciate the feedback and the proposal!

@jonabc
Copy link
Contributor

jonabc commented Feb 25, 2023

I opened #633 to continue the conversation thread about which configurations should be included as a default.

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