Do not declare project dependencies if there is no code to generate #1544
Conversation
Generate changelog in
|
|
|
||
| private static void configureProjectDependencies(Project project) { | ||
| project.getDependencies().add("api", "com.palantir.tritium:tritium-registry"); | ||
| project.getDependencies().add("api", "com.palantir.safe-logging:preconditions"); |
There was a problem hiding this comment.
Pretty sure this also should be an implementation dependency, but I didn't want to change it just here, due to it possibly already being used transitively in downstream projects.
|
|
||
| configureProjectDependencies(project); | ||
| // Only add dependencies if we're going to generate code | ||
| if (!metricSchemaSourceDirectorySet.getFiles().isEmpty()) { |
There was a problem hiding this comment.
I'm unsure whether this is the proper way to do this. Is there a lazy way to do this with providers or something? (I wouldn't think so due to it being for dependency declaration, but checking just in case)
There was a problem hiding this comment.
I think because the whole configuration is setup above this should be fine, so long as nothing is dynamically generating sources at a later point?
There was a problem hiding this comment.
This initially feels not ideal to me, as it's checking where the files exist immediately at configuration time rather than being lazy as possible? It would be bad if this were generated code, perhaps acceptable if this is human authored code (not really sure without diving in metric-schema).
Anyway, the way to do this lazily is with this:
project.getDependencies().addProvider(...)I'd honestly just do this as this is more likely to be correct.
There was a problem hiding this comment.
Actually, you can't use that addProvider to conditionally not a dependency.
Instead you want:
project.getConfigurations().named("api", conf -> {
conf.getDependencies().addAllLater(project.provider(() ->
/* return a list/set of dependencies made with project.getDependencies().create(...) */);
})There was a problem hiding this comment.
This is checking for manually-authored metrics.yml files in the custom sourceset that is defined above, containing the metrics definitions to generate code from, so this should be mostly ok.
✅ Successfully generated changelog entry!Need to regenerate?Simply interact with the changelog bot comment again to regenerate these entries. 📋Changelog Preview🐛 Fixes
|
|
Released 0.35.0 |
Before this PR
#1443 introduced two missing dependencies. However, because they were declared as
implementation, this caused some setups to fail thecheckUnusedDependenciestasks, in the case where there ismetrics.ymlfile and thus no code to generate and use the dependencies (this setup exists in services that still generate themetrics.mdfile even if they themselves don't add new metrics)After this PR
==COMMIT_MSG==
Do not declare project dependencies if there is no code to generate
==COMMIT_MSG==
It also turns out that
metrics-corewas actually part of the public API of the generated code, so I updated it to be declared as API rather than implementation.Possible downsides?