royteeuwen commented on code in PR #56:
URL:
https://github.com/apache/sling-org-apache-sling-committer-cli/pull/56#discussion_r3890396825
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateDistCommand.java:
##########
@@ -169,22 +189,50 @@ record DistReleasePlan(
* Shared by this command and {@link FinalizeCommand} so the flow is not
duplicated. When the version is
* already present in {@code dist/release} the returned plan is marked
{@link DistReleasePlan#alreadyPublished()}.
*/
- static DistReleasePlan planDistRelease(
+ static List<DistReleasePlan> planDistRelease(
RepositoryService repositoryService, StagingRepository repository,
String previousVersion)
throws IOException {
LocalRepository localRepository =
repositoryService.download(repository);
- Artifact primary = localRepository.getArtifacts().stream()
+ List<Artifact> artifacts = localRepository.getArtifacts().stream()
.filter(a -> "pom".equals(a.getType()))
- .findFirst()
- .orElseThrow(() -> new IllegalStateException("No POM artifact
found in staging repository"));
- String artifactId = primary.getArtifactId();
- String newVersion = primary.getVersion();
- if (DistRepository.isVersionPublished(artifactId, newVersion)) {
- return new DistReleasePlan(artifactId, newVersion, List.of(),
List.of(), true);
+ .toList();
+
+ if (artifacts.isEmpty()) {
+ throw new IllegalStateException("No POM artifact found in staging
repository");
+ }
+
+ List<DistReleasePlan> plans = artifacts.stream()
+ .filter(a -> {
+ try {
+ return
DistRepository.isVersionPublished(a.getArtifactId(), a.getVersion());
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ })
+ .map(a -> new DistReleasePlan(a.getArtifactId(),
a.getVersion(), List.of(), List.of(), true))
+ .toList();
+
+ if (!plans.isEmpty()) {
Review Comment:
**Blocking: partial publishes become permanently unrecoverable.**
This filter keeps only the artifacts that are *already* published, and then
`if (!plans.isEmpty()) return plans;` throws away every artifact that still
needs publishing. So if even one artifact of a multi-artifact release is
already in `dist/release`, the others are silently dropped from the plan — and
`doUpdateDist`'s `plans.stream().allMatch(DistReleasePlan::alreadyPublished)`
is then trivially true, so it logs "nothing to do" and exits `OK`.
This is exactly the state you land in after a partially-failed run (see my
comment on the per-plan `DistRepository.publish` call): artifact A committed,
artifact B failed → re-run `update-dist` → "nothing to do", forever, with no
way to recover other than doing it by hand.
The whole point of `isVersionPublished` was to make re-runs safe and this
inverts it. I'd suggest a single pass that builds one plan per artifact with
its own `alreadyPublished` flag, and then has `doUpdateDist` skip the published
ones and publish the rest.
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateDistCommand.java:
##########
@@ -169,22 +189,50 @@ record DistReleasePlan(
* Shared by this command and {@link FinalizeCommand} so the flow is not
duplicated. When the version is
* already present in {@code dist/release} the returned plan is marked
{@link DistReleasePlan#alreadyPublished()}.
*/
- static DistReleasePlan planDistRelease(
+ static List<DistReleasePlan> planDistRelease(
RepositoryService repositoryService, StagingRepository repository,
String previousVersion)
throws IOException {
LocalRepository localRepository =
repositoryService.download(repository);
- Artifact primary = localRepository.getArtifacts().stream()
+ List<Artifact> artifacts = localRepository.getArtifacts().stream()
.filter(a -> "pom".equals(a.getType()))
- .findFirst()
- .orElseThrow(() -> new IllegalStateException("No POM artifact
found in staging repository"));
- String artifactId = primary.getArtifactId();
- String newVersion = primary.getVersion();
- if (DistRepository.isVersionPublished(artifactId, newVersion)) {
- return new DistReleasePlan(artifactId, newVersion, List.of(),
List.of(), true);
+ .toList();
+
+ if (artifacts.isEmpty()) {
+ throw new IllegalStateException("No POM artifact found in staging
repository");
+ }
+
+ List<DistReleasePlan> plans = artifacts.stream()
+ .filter(a -> {
+ try {
+ return
DistRepository.isVersionPublished(a.getArtifactId(), a.getVersion());
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ })
+ .map(a -> new DistReleasePlan(a.getArtifactId(),
a.getVersion(), List.of(), List.of(), true))
+ .toList();
+
+ if (!plans.isEmpty()) {
+ return plans;
+ } else {
+ return artifacts.stream()
+ .map(a -> {
+ try {
+ String artifactId = a.getArtifactId();
+ String newVersion = a.getVersion();
+ List<Path> newFiles =
collectDownloadedFiles(localRepository.getRootFolder()).stream()
+ .filter(path ->
+
path.getFileName().toString().startsWith(artifactId + "-" + newVersion))
Review Comment:
**Blocking: `startsWith` reintroduces the version-prefix bug.**
Publishing `1.0.14` will pick up `1.0.140`'s files here.
`DistRepository.belongsToVersion` already handles exactly this (there's even a
test guarding it —
`testAutoDeduceDoesNotConfuseVersionPrefixesAndKeepsNewerVersions`); it's just
`private`. Could you widen it to package-visible and reuse it instead?
Separately, a behaviour change worth confirming is intentional: previously
*every* downloaded file was published, now only files matching some pom's
`artifactId-version` prefix are, and anything that doesn't match is silently
dropped.
##########
src/main/java/org/apache/sling/cli/impl/release/FinalizeCommand.java:
##########
@@ -305,39 +304,8 @@ private void stepPromoteStage(StagingRepository
repository, ExecutionMode mode)
}
}
- private void stepUpdateDist(StagingRepository repository, ExecutionMode
mode) throws IOException {
- // Delegate the download/collect/publish flow to UpdateDistCommand so
it is not duplicated here.
- UpdateDistCommand.DistReleasePlan plan =
UpdateDistCommand.planDistRelease(repositoryService, repository, null);
-
- if (plan.alreadyPublished()) {
- LOGGER.info("dist/release already contains {} {}; skipping.",
plan.artifactId(), plan.newVersion());
- return;
- }
- if (plan.newFiles().isEmpty()) {
- LOGGER.warn(
- "No artifacts were downloaded for {} {}; skipping dist
update.",
- plan.artifactId(),
- plan.newVersion());
- return;
- }
-
- if (mode == ExecutionMode.DRY_RUN) {
- LOGGER.info(
- "Would publish {} file(s) to dist/release for {} {}",
- plan.newFiles().size(),
- plan.artifactId(),
- plan.newVersion());
- LOGGER.info(
- "Would remove {} old file(s) from dist/release",
- plan.oldFiles().size());
- } else {
- DistRepository.publish(
- plan.artifactId(),
- plan.newVersion(),
- plan.newFiles(),
- plan.oldFiles(),
- credentialsService.getAsfCredentials());
- }
+ private void stepUpdateDist(ExecutionMode mode) throws IOException {
Review Comment:
**Blocking: the exit code is discarded.**
`doUpdateDist` swallows `IOException`, logs a warning and returns
`ExitCode.SOFTWARE` — which is thrown away here, so finalize continues straight
on to **Step 2: Promote to Maven Central** as though dist succeeded. Previously
the `IOException` from `DistRepository.publish` propagated out of
`stepUpdateDist` and aborted finalize *before* the irreversible promote, which
is the one ordering that really matters here.
Also: `repository` is already resolved and passed into
`stepUpdateDistStage`, but passing `repositoryId` here makes `doUpdateDist`
call `repositoryService.find()` and download the whole staging repository a
second time.
##########
src/test/java/org/apache/sling/cli/impl/release/UpdateDistCommandTest.java:
##########
@@ -539,4 +582,15 @@ private Command createCommand(ExecutionMode executionMode,
String previousVersio
result instanceof UpdateDistCommand);
return result;
}
+
+ private record ArtifactUpdate(String artifactId, String newVersion, String
oldVersion) {
+ String oldArtifact() {
+ return artifactId() + "-" + oldVersion();
+ }
+
+ String newArtifact() {
+ return artifactId() + "-" + newVersion();
+ }
+ }
+ ;
Review Comment:
Stray `;` after the record declaration. Also `newArtifact()` above is unused.
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateDistCommand.java:
##########
@@ -90,70 +93,87 @@ public class UpdateDistCommand implements Command {
@Override
public Integer call() {
+ Integer ok = doUpdateDist(
+ repositoryService, repositoryId, previousVersion,
reusableCLIOptions.executionMode, credentialsService);
+ if (ok != null) return ok;
+ return CommandLine.ExitCode.OK;
+ }
+
+ public static @Nullable Integer doUpdateDist(
Review Comment:
The `@Nullable Integer` contract where `null` means success is hard to read
— `call()` ends up doing `if (ok != null) return ok; return
CommandLine.ExitCode.OK;`, and both `null` and `ExitCode.OK` mean OK. Could
this just return the exit code directly?
Also, `planDistRelease` still declares `throws IOException` but now leaks
`UncheckedIOException` from the stream lambdas instead (hence the `catch
(UncheckedIOException | IOException)` here). Plain `for` loops in
`planDistRelease` would be shorter and keep the checked exception honest.
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateDistCommand.java:
##########
@@ -90,70 +93,87 @@ public class UpdateDistCommand implements Command {
@Override
public Integer call() {
+ Integer ok = doUpdateDist(
+ repositoryService, repositoryId, previousVersion,
reusableCLIOptions.executionMode, credentialsService);
+ if (ok != null) return ok;
+ return CommandLine.ExitCode.OK;
+ }
+
+ public static @Nullable Integer doUpdateDist(
+ RepositoryService repositoryService,
+ Integer repositoryId,
+ String previousVersion,
+ ExecutionMode executionMode,
+ CredentialsService credentialsService) {
try {
- DistReleasePlan plan =
+ List<DistReleasePlan> plans =
planDistRelease(repositoryService,
repositoryService.find(repositoryId), previousVersion);
- if (plan.alreadyPublished()) {
- LOGGER.info(
- "dist/release already contains {} {}; nothing to do.",
plan.artifactId(), plan.newVersion());
+ if (plans.stream().allMatch(DistReleasePlan::alreadyPublished)) {
+ for (DistReleasePlan plan : plans) {
+ LOGGER.info(
+ "dist/release already contains {} {}; nothing to
do.",
+ plan.artifactId(),
+ plan.newVersion());
+ }
return CommandLine.ExitCode.OK;
}
- if (plan.newFiles().isEmpty()) {
+
+ boolean noArtifacts = plans.stream()
+ .flatMap(plan -> plan.newFiles().stream())
+ .findFirst()
+ .isEmpty();
+ if (noArtifacts) {
Review Comment:
**Blocking: this can delete an artifact's previous release without
publishing the new one.**
`noArtifacts` is now true only if *every* plan has empty `newFiles`. If
artifact A has staged files and B doesn't, B's plan still reaches
`DistRepository.publish(id, ver, List.of(), oldFiles, ...)` below — and
`commitFiles` has no guard for an empty `newFiles`, so it happily deletes
`oldFiles` and adds nothing. B's previous release is then gone from
`dist/release`.
The `newFiles().isEmpty()` check needs to stay per-plan (skip that plan, or
fail the whole command).
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateDistCommand.java:
##########
@@ -90,70 +93,87 @@ public class UpdateDistCommand implements Command {
@Override
public Integer call() {
+ Integer ok = doUpdateDist(
+ repositoryService, repositoryId, previousVersion,
reusableCLIOptions.executionMode, credentialsService);
+ if (ok != null) return ok;
+ return CommandLine.ExitCode.OK;
+ }
+
+ public static @Nullable Integer doUpdateDist(
+ RepositoryService repositoryService,
+ Integer repositoryId,
+ String previousVersion,
+ ExecutionMode executionMode,
+ CredentialsService credentialsService) {
try {
- DistReleasePlan plan =
+ List<DistReleasePlan> plans =
planDistRelease(repositoryService,
repositoryService.find(repositoryId), previousVersion);
- if (plan.alreadyPublished()) {
- LOGGER.info(
- "dist/release already contains {} {}; nothing to do.",
plan.artifactId(), plan.newVersion());
+ if (plans.stream().allMatch(DistReleasePlan::alreadyPublished)) {
+ for (DistReleasePlan plan : plans) {
+ LOGGER.info(
+ "dist/release already contains {} {}; nothing to
do.",
+ plan.artifactId(),
+ plan.newVersion());
+ }
return CommandLine.ExitCode.OK;
}
- if (plan.newFiles().isEmpty()) {
+
+ boolean noArtifacts = plans.stream()
+ .flatMap(plan -> plan.newFiles().stream())
+ .findFirst()
+ .isEmpty();
+ if (noArtifacts) {
LOGGER.warn("No artifacts were downloaded for staging
repository {}.", repositoryId);
return CommandLine.ExitCode.USAGE;
}
- switch (reusableCLIOptions.executionMode) {
- case DRY_RUN:
- LOGGER.info(
- "Would publish {} file(s) to dist/release for {}
{}:",
- plan.newFiles().size(),
- plan.artifactId(),
- plan.newVersion());
- plan.newFiles()
- .forEach(f -> LOGGER.info(
- " put {} -> {}{}", f,
DistRepository.DIST_RELEASE_URL, f.getFileName()));
- if (!plan.oldFiles().isEmpty()) {
- LOGGER.info(
- "Would remove {} old file(s) from
dist/release:",
- plan.oldFiles().size());
- plan.oldFiles().forEach(f -> LOGGER.info(" rm {}",
DistRepository.DIST_RELEASE_URL + f));
- }
- break;
- case INTERACTIVE:
+ for (DistReleasePlan plan : plans) {
+ boolean doPerformPublish = executionMode == ExecutionMode.AUTO;
+ if (executionMode == ExecutionMode.INTERACTIVE) {
String question = String.format(
"Publish %d file(s) for %s %s to dist/release and
remove %d older file(s) for %s?",
plan.newFiles().size(),
plan.artifactId(),
plan.newVersion(),
plan.oldFiles().size(),
plan.artifactId());
- if (InputOption.YES.equals(UserInput.yesNo(question,
InputOption.YES))) {
- DistRepository.publish(
- plan.artifactId(),
- plan.newVersion(),
- plan.newFiles(),
- plan.oldFiles(),
- credentialsService.getAsfCredentials());
- } else {
+ doPerformPublish =
InputOption.YES.equals(UserInput.yesNo(question, InputOption.YES));
+ if (!doPerformPublish) {
LOGGER.info("Aborted.");
}
- break;
- case AUTO:
+ }
+
+ if (doPerformPublish) {
Review Comment:
**Blocking (see summary item 3):** calling `publish` once per plan turns a
multi-artifact release into N separate SVN commits on dist.apache.org.
`DistRepository.publish`'s javadoc promises "a single atomic revision", and
`commitFiles` aborts the edit on failure specifically so a partial commit is
never left behind — that guarantee no longer holds for the release as a whole.
Collecting all plans' `newFiles`/`oldFiles` into one `publish` call would
keep it atomic (the commit message would need to name the set rather than a
single `artifactId version`).
##########
src/test/java/org/apache/sling/cli/impl/release/FinalizeCommandTest.java:
##########
@@ -159,12 +159,12 @@ public void testDryRunPmc() throws Exception {
try (MockedStatic<UpdateDistCommand> dist =
mockStatic(UpdateDistCommand.class);
MockedStatic<DistRepository> distRepo =
mockStatic(DistRepository.class)) {
dist.when(() -> UpdateDistCommand.planDistRelease(any(), any(),
any()))
- .thenReturn(new UpdateDistCommand.DistReleasePlan(
+ .thenReturn(List.of(new UpdateDistCommand.DistReleasePlan(
Review Comment:
`testDryRunPmc` is vacuous now: with `mockStatic(UpdateDistCommand.class)`
and no `thenCallRealMethod()` for `doUpdateDist`, the entire dist step is a
no-op — this `planDistRelease` stub is never reached and the
`verify(DistRepository.publish, never())` below passes trivially. `testAutoPmc`
got the `thenCallRealMethod()`; this test needs it too.
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateDistCommand.java:
##########
@@ -90,70 +93,87 @@ public class UpdateDistCommand implements Command {
@Override
public Integer call() {
+ Integer ok = doUpdateDist(
+ repositoryService, repositoryId, previousVersion,
reusableCLIOptions.executionMode, credentialsService);
+ if (ok != null) return ok;
+ return CommandLine.ExitCode.OK;
+ }
+
+ public static @Nullable Integer doUpdateDist(
+ RepositoryService repositoryService,
+ Integer repositoryId,
+ String previousVersion,
+ ExecutionMode executionMode,
+ CredentialsService credentialsService) {
try {
- DistReleasePlan plan =
+ List<DistReleasePlan> plans =
planDistRelease(repositoryService,
repositoryService.find(repositoryId), previousVersion);
- if (plan.alreadyPublished()) {
- LOGGER.info(
- "dist/release already contains {} {}; nothing to do.",
plan.artifactId(), plan.newVersion());
+ if (plans.stream().allMatch(DistReleasePlan::alreadyPublished)) {
+ for (DistReleasePlan plan : plans) {
+ LOGGER.info(
+ "dist/release already contains {} {}; nothing to
do.",
+ plan.artifactId(),
+ plan.newVersion());
+ }
return CommandLine.ExitCode.OK;
}
- if (plan.newFiles().isEmpty()) {
+
+ boolean noArtifacts = plans.stream()
+ .flatMap(plan -> plan.newFiles().stream())
+ .findFirst()
+ .isEmpty();
+ if (noArtifacts) {
LOGGER.warn("No artifacts were downloaded for staging
repository {}.", repositoryId);
return CommandLine.ExitCode.USAGE;
}
- switch (reusableCLIOptions.executionMode) {
- case DRY_RUN:
- LOGGER.info(
- "Would publish {} file(s) to dist/release for {}
{}:",
- plan.newFiles().size(),
- plan.artifactId(),
- plan.newVersion());
- plan.newFiles()
- .forEach(f -> LOGGER.info(
- " put {} -> {}{}", f,
DistRepository.DIST_RELEASE_URL, f.getFileName()));
- if (!plan.oldFiles().isEmpty()) {
- LOGGER.info(
- "Would remove {} old file(s) from
dist/release:",
- plan.oldFiles().size());
- plan.oldFiles().forEach(f -> LOGGER.info(" rm {}",
DistRepository.DIST_RELEASE_URL + f));
- }
- break;
- case INTERACTIVE:
+ for (DistReleasePlan plan : plans) {
+ boolean doPerformPublish = executionMode == ExecutionMode.AUTO;
+ if (executionMode == ExecutionMode.INTERACTIVE) {
String question = String.format(
"Publish %d file(s) for %s %s to dist/release and
remove %d older file(s) for %s?",
plan.newFiles().size(),
plan.artifactId(),
plan.newVersion(),
plan.oldFiles().size(),
plan.artifactId());
- if (InputOption.YES.equals(UserInput.yesNo(question,
InputOption.YES))) {
- DistRepository.publish(
- plan.artifactId(),
- plan.newVersion(),
- plan.newFiles(),
- plan.oldFiles(),
- credentialsService.getAsfCredentials());
- } else {
+ doPerformPublish =
InputOption.YES.equals(UserInput.yesNo(question, InputOption.YES));
+ if (!doPerformPublish) {
Review Comment:
Declining an interactive prompt now falls into the `else` branch below, so
the user sees `Aborted.` immediately followed by `Would publish 5 file(s) to
dist/release for ...` — the dry-run text. DRY_RUN and "user said no" probably
shouldn't share a branch.
Also worth considering: declining one artifact silently continues prompting
for the next, and there's no summary at the end of what was skipped.
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateDistCommand.java:
##########
@@ -169,22 +189,50 @@ record DistReleasePlan(
* Shared by this command and {@link FinalizeCommand} so the flow is not
duplicated. When the version is
* already present in {@code dist/release} the returned plan is marked
{@link DistReleasePlan#alreadyPublished()}.
*/
- static DistReleasePlan planDistRelease(
+ static List<DistReleasePlan> planDistRelease(
RepositoryService repositoryService, StagingRepository repository,
String previousVersion)
throws IOException {
LocalRepository localRepository =
repositoryService.download(repository);
- Artifact primary = localRepository.getArtifacts().stream()
+ List<Artifact> artifacts = localRepository.getArtifacts().stream()
.filter(a -> "pom".equals(a.getType()))
- .findFirst()
- .orElseThrow(() -> new IllegalStateException("No POM artifact
found in staging repository"));
- String artifactId = primary.getArtifactId();
- String newVersion = primary.getVersion();
- if (DistRepository.isVersionPublished(artifactId, newVersion)) {
- return new DistReleasePlan(artifactId, newVersion, List.of(),
List.of(), true);
+ .toList();
+
+ if (artifacts.isEmpty()) {
+ throw new IllegalStateException("No POM artifact found in staging
repository");
+ }
+
+ List<DistReleasePlan> plans = artifacts.stream()
+ .filter(a -> {
+ try {
+ return
DistRepository.isVersionPublished(a.getArtifactId(), a.getVersion());
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ })
+ .map(a -> new DistReleasePlan(a.getArtifactId(),
a.getVersion(), List.of(), List.of(), true))
+ .toList();
+
+ if (!plans.isEmpty()) {
+ return plans;
+ } else {
+ return artifacts.stream()
+ .map(a -> {
+ try {
+ String artifactId = a.getArtifactId();
+ String newVersion = a.getVersion();
+ List<Path> newFiles =
collectDownloadedFiles(localRepository.getRootFolder()).stream()
Review Comment:
Minor: `collectDownloadedFiles` walks the entire download tree once per
artifact. Worth hoisting the call out of the `map` and filtering the single
result per artifact.
##########
src/main/java/org/apache/sling/cli/impl/release/FinalizeCommand.java:
##########
@@ -288,7 +287,7 @@ private void stepUpdateDistStage(StagingRepository
repository, ExecutionMode mod
LOGGER.info("SKIPPED (staging repository already promoted; if dist
still needs updating a PMC"
+ " member must run update-dist separately)");
} else {
- stepUpdateDist(repository, mode);
+ stepUpdateDist(reusableCLIOptions.executionMode);
Review Comment:
Nit: `stepUpdateDistStage` receives `mode` and it's now unused — this
reaches for `reusableCLIOptions.executionMode` instead. Same value today, but
inconsistent with the rest of the class (`stepPromoteStage`, `stepUpdateSite`,
… all use the parameter).
Worth calling out too: the old `stepUpdateDist` had no INTERACTIVE branch
and published directly, so finalize will now prompt per artifact mid-run.
Probably an improvement, but it's an unannounced behaviour change.
##########
src/test/java/org/apache/sling/cli/impl/release/UpdateDistCommandTest.java:
##########
@@ -91,7 +92,7 @@ public void
testAutoDeducePreviousFilesExcludesNewVersionAndSiblings() throws Ex
ARTIFACT + "-1.3.4.pom.asc",
ARTIFACT + "-1.3.4-source-release.zip",
ARTIFACT + "-1.3.4-source-release.zip.asc",
- ARTIFACT + "-1.3.6.pom", // the new version - must be kept
(not removed)
+ ARTIFACT + "-1.3.6.pom", // the new newVersion - must be kept
(not removed)
Review Comment:
This looks like an accidental rename-in-comments: "the new **newVersion**".
It recurs about ten times through this file (lines ~95, 108, 118, 124, 127,
206, 261, 269-270, 273, 386) and makes the comments read wrong. Could you
revert the comment/assertion-message churn so the diff stays focused?
##########
src/test/java/org/apache/sling/cli/impl/release/UpdateDistCommandTest.java:
##########
@@ -202,7 +203,49 @@ public void
testAutoDeduceKeepsOlderMajorWhenNoSameMajorPredecessorExists() thro
List<String> old =
DistRepository.listPreviousReleaseFiles(ARTIFACT, "2.0.0", null);
- assertTrue("a different major version must never be removed",
old.isEmpty());
+ assertTrue("a different major newVersion must never be removed",
old.isEmpty());
+ }
+ }
+
+ @Test
+ public void testAutoDeduceIdentifiesMultiArtifactReleases() throws
Exception {
Review Comment:
This test doesn't actually exercise the new code — it only calls
`DistRepository.listPreviousReleaseFiles`, which this PR doesn't touch. It
never goes through `planDistRelease` with multiple artifacts, nor
`doUpdateDist` over multiple plans. Both `ArtifactUpdate` entries also share
the same `artifactId` with different versions, so it isn't really a
multi-*artifact* release either.
The cases I'd most like to see covered, since they're the ones that bite in
production:
- multiple pom artifacts in the staging repo → one plan each, correct
`newFiles` split between them
- one artifact already published + one not → the unpublished one still gets
published (currently broken)
- one artifact with no staged files → its `oldFiles` are *not* deleted
(currently broken)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
