Improve project resolution of nested subprojects for jar manifests #5618
Conversation
ghost
requested review from
gradle/publish-java.gradle
Outdated
| projectDependencies.removeAll(collect.collect {it-".jar"}) | ||
| // projectDependencies.removeAll(collect.collect {it-".jar"}) | ||
| } catch (UnknownProjectException ignore) { | ||
| throw ignore |
There was a problem hiding this comment.
If we throw the exception again, without any action, why not take out the try/catch block?
There was a problem hiding this comment.
I think that exception isn't possible there anyway. Pull it.
There was a problem hiding this comment.
Because I forgot, while we were pairing.
gradle/publish-java.gradle
Outdated
| projectDependencies.removeAll(collect.collect {it-".jar"}) | ||
| // projectDependencies.removeAll(collect.collect {it-".jar"}) | ||
| } catch (UnknownProjectException ignore) { | ||
| throw ignore |
There was a problem hiding this comment.
I think that exception isn't possible there anyway. Pull it.
gradle/publish-java.gradle
Outdated
| def parentProject = project(":$geodeProject" - "-$version") | ||
| def parentProject = geodeProject | ||
| def collect = parentProject.configurations.runtimeClasspath.collect { it.name } | ||
| runtimeList.removeAll(collect) |
There was a problem hiding this comment.
I really think we don't want to be removing these dependencies. If we discovered them in the block above as direct dependencies they should be included. Because we aren't iterating over transitive dependencies we shouldn't have any to filter out.
There was a problem hiding this comment.
@kohlmu-pivotal Can we get consensus on these remove statements before converting this draft to a final PR?
There was a problem hiding this comment.
I believe this to be correct for the state of the project at this stage. Until libraries are loaded as modules and ensures that there is ever only one copy of a class at a particular version, or at least not have some classes cross module boundaries, this should stay.
We can always change in the future, but for now, this is sufficient.
|
@pivotal-jbarrett Is that logic better? |
|
Creating non-draft PR -> #5660 |
ghost
closed this
3 participants
No description provided.