Conversation
vjovanov
force-pushed
the
vj/fix-infra-testing
branch
2 times, most recently
from
c624325 to
51e8c2d
Compare
vjovanov
force-pushed
the
vj/fix-infra-testing
branch
5 times, most recently
from
e30ec27 to
22f721e
Compare
vjovanov
force-pushed
the
vj/fix-infra-testing
branch
4 times, most recently
from
54cedc8 to
95ffef6
Compare
vjovanov
force-pushed
the
vj/fix-infra-testing
branch
from
cccfa78 to
c4c8e05
Compare
jormundur00
left a comment
jormundur00
left a comment
There was a problem hiding this comment.
LGTM! Didn't test every possible combination of commands locally, but what I did worked (and I see that all the changed-build-logic tests passed in the CI, so we should be good).
| registerPerCoordinateTask(coordinates, javaTest, "javaTest", JavaTestInvocationTask, true) | ||
| registerPerCoordinateTask(coordinates, nativeTestCompile, "nativeTestCompile", NativeTestCompileInvocationTask, true) | ||
| // No aggregate wiring for per-coordinate docker pulls; they are invoked by dedicated workflows. | ||
| registerPerCoordinateTask(coordinates, null, "pullAllowedDockerImages", DockerTask.class, false) { t -> |
There was a problem hiding this comment.
With the addition of ComputeAndPullAllowedDockerImagesTask, I don't think DockerTask is used anywhere (you removed it here, but it's still an unused import). I think we can consider deleting it from the repository, but we can do that in a follow-up PR to avoid rerunning all tests for this change.
| protected String resolveSingleCoordinate() { | ||
| List<String> coords = resolveCoordinates() | ||
| if (coords.isEmpty()) { | ||
| throw new GradleException("No matching coordinates found. Provide a concrete coordinate via -Pcoordinates=group:artifact:version") |
There was a problem hiding this comment.
Small nit: in the root build.gradle, when throwing the GradleException, the message states to use -Pcoordinate (or -Pcoordinates in brackets), but here we ask for -Pcoordinates explicitly. If we intend to have -Pcoordinate as a valid property, maybe we should add and use it here as well (as a fallback at least). We can also do this in a follow-up, or just ignore it.
There was a problem hiding this comment.
I will fix this up later, as this PR runs for a long long time.
2 participants
No description provided.