rombert commented on code in PR #40:
URL:
https://github.com/apache/sling-org-apache-sling-committer-cli/pull/40#discussion_r3756860314
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateDistCommand.java:
##########
@@ -390,6 +390,17 @@ private static boolean belongsToVersion(String fileName,
String artifactId, Stri
return next == '.' || next == '-';
}
+ /**
+ * Returns the names of every {@code .pom} published in {@code
dist/release} for {@code version}, across
+ * all artifacts. Used to resolve a release's artifact ids from the
released POMs once the staging
+ * repository is gone; the caller narrows the candidates down by reading
each POM's {@code <name>}.
+ */
+ static List<String> listReleasePomFileNames(String version) throws
IOException {
Review Comment:
This is a smell that we need to move some code out of the
`UpdateDistCommand` - reusable code should live in another class.
##########
src/test/java/org/apache/sling/cli/impl/release/UpdateNewsCommandTest.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.cli.impl.release;
+
+import java.io.IOException;
+import java.util.Set;
+
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.sling.cli.impl.Command;
+import org.apache.sling.cli.impl.Credentials;
+import org.apache.sling.cli.impl.CredentialsService;
+import org.apache.sling.cli.impl.ExecutionMode;
+import org.apache.sling.cli.impl.jbake.JBakeContentUpdater;
+import org.apache.sling.cli.impl.junit.LogCapture;
+import org.apache.sling.cli.impl.nexus.RepositoryService;
+import org.apache.sling.cli.impl.nexus.StagingRepository;
+import org.apache.sling.cli.impl.people.Member;
+import org.apache.sling.cli.impl.people.MembersFinder;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
+import org.eclipse.jgit.api.AddCommand;
+import org.eclipse.jgit.api.CloneCommand;
+import org.eclipse.jgit.api.CommitCommand;
+import org.eclipse.jgit.api.DiffCommand;
+import org.eclipse.jgit.api.FetchCommand;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.PushCommand;
+import org.eclipse.jgit.api.ResetCommand;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.mockito.MockedConstruction;
+import org.mockito.MockedStatic;
+import picocli.CommandLine;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockConstruction;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class UpdateNewsCommandTest {
+
+ @Rule
+ public final OsgiContext osgiContext = new OsgiContext();
+
+ @Rule
+ public final LogCapture logCapture = new
LogCapture(UpdateNewsCommand.class);
+
+ @Rule
+ public final TemporaryFolder checkout = new TemporaryFolder();
+
+ /** Keeps the site checkout inside the test's temporary folder rather than
the real user's home. */
+ @Before
+ public void redirectCheckout() {
+ System.setProperty(
+ UpdateLocalSiteCommand.CHECKOUT_PROPERTY,
checkout.getRoot().getAbsolutePath());
+ }
+
+ @After
+ public void restoreCheckout() {
+ System.clearProperty(UpdateLocalSiteCommand.CHECKOUT_PROPERTY);
+ }
+
+ private PushCommand pushCommand;
+ private CommitCommand commitCommand;
+
+ /** Stubs out JGit so nothing is cloned, opened, reset or pushed for real.
*/
+ private MockedStatic<Git> stubGit() {
Review Comment:
There is some heavy mocking done here and also in
https://github.com/apache/sling-org-apache-sling-committer-cli/pull/40/changes#diff-bb658e24600352f9b0bf075f38bb9021316945bf351355c9a9a76a2d5da8e4c3R100
. Have you considered alternatives?
##########
src/main/java/org/apache/sling/cli/impl/DateProvider.java:
##########
@@ -30,7 +31,7 @@ public class DateProvider {
private static final DateTimeFormatter jiraReleaseDate =
DateTimeFormatter.ofPattern("yyyy-MM-d");
public OffsetDateTime getCurrentDate() {
- return OffsetDateTime.now();
+ return OffsetDateTime.now(ZoneId.systemDefault());
Review Comment:
This seems unrelated, why is it needed?
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateNewsCommand.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.cli.impl.release;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.sling.cli.impl.Command;
+import org.apache.sling.cli.impl.CredentialsService;
+import org.apache.sling.cli.impl.jbake.JBakeContentUpdater;
+import org.apache.sling.cli.impl.nexus.RepositoryService;
+import org.apache.sling.cli.impl.people.MembersFinder;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import picocli.CommandLine;
+
+/**
+ * Announces a release on the Sling website's news page.
+ *
+ * <p>Deliberately <em>not</em> part of {@link FinalizeCommand}: the release
management guide only asks for a
+ * news entry when a release warrants an announcement, which is a judgement
call, and most module releases do
+ * not get one. Run this by hand for the releases that do.
+ */
+@Component(
Review Comment:
Cool, nice that you added this :-)
##########
src/main/java/org/apache/sling/cli/impl/release/UpdateLocalSiteCommand.java:
##########
@@ -46,63 +76,289 @@
})
@CommandLine.Command(
name = UpdateLocalSiteCommand.NAME,
- description = "Updates the Sling website with the new release
information, " + "based on a local checkout",
+ description = "Updates the Sling website with the new release
information, based on a local checkout",
subcommands = CommandLine.HelpCommand.class)
public class UpdateLocalSiteCommand extends AbstractReleaseCommand {
static final String GROUP = "release";
static final String NAME = "update-local-site";
- private static final String GIT_CHECKOUT = "/tmp/sling-site";
+ /** System property, or {@code SLING_CLI_SITE_CHECKOUT} environment
variable, overriding the checkout. */
+ static final String CHECKOUT_PROPERTY = "sling.cli.site.checkout";
+
+ /** Cloned over https from gitbox so the same ASF credentials that commit
to dist.apache.org can push. */
+ static final String SITE_GIT_URL =
"https://gitbox.apache.org/repos/asf/sling-site.git";;
+
+ /**
+ * Where the site is checked out. Deliberately not under the shared
temporary directory: the checkout is
+ * reused across runs and is committed from, so a world-writable location
would let anything else on the
+ * host substitute the content that gets pushed to the website. Resolved
per call so it can be redirected.
+ */
+ static String checkoutDir() {
+ String configured = System.getProperty(CHECKOUT_PROPERTY);
+ if (configured == null || configured.isBlank()) {
+ configured = System.getenv("SLING_CLI_SITE_CHECKOUT");
Review Comment:
Why an env var and not a regular CLI argument?
--
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]
