Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webrev: add logging #674

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -23,6 +23,7 @@
module org.openjdk.skara.webrev {
requires org.openjdk.skara.vcs;
requires java.net.http;
requires java.logging;

exports org.openjdk.skara.webrev;
}
@@ -50,20 +50,25 @@ public ModifiedFileView(ReadOnlyRepository repo, Hash base, Hash head, List<Comm
this.formatter = formatter;
if (patch.isTextual()) {
binaryContent = null;
oldContent = repo.lines(patch.source().path().get(), base).orElseThrow(() -> {
throw new IllegalArgumentException("Could not get content for file " +
patch.source().path().get() +
" at revision " + base);
});
var sourcePath = patch.source().path().orElseThrow(() ->
new IllegalArgumentException("Could not get source path for file with hash " +
patch.source().hash() + " with target path" +
patch.target().path().get())
edvbld marked this conversation as resolved.
Show resolved Hide resolved
);

oldContent = repo.lines(sourcePath, base).orElseThrow(() ->
new IllegalArgumentException("Could not get content for file " +
sourcePath + " at revision " + base)
);
if (head == null) {
var path = repo.root().resolve(patch.target().path().get());
if (patch.target().type().get().isVCSLink()) {
var tip = repo.head();
var content = repo.lines(patch.target().path().get(), tip).orElseThrow(() -> {
throw new IllegalArgumentException("Could not get content for file " +
var content = repo.lines(patch.target().path().get(), tip).orElseThrow(() ->
new IllegalArgumentException("Could not get content for file " +
patch.target().path().get() +
" at revision " + tip);
});
" at revision " + tip)
);
newContent = List.of(content.get(0) + "-dirty");
} else {
List<String> lines = null;
@@ -81,11 +86,11 @@ public ModifiedFileView(ReadOnlyRepository repo, Hash base, Hash head, List<Comm
newContent = lines;
}
} else {
newContent = repo.lines(patch.target().path().get(), head).orElseThrow(() -> {
throw new IllegalArgumentException("Could not get content for file " +
newContent = repo.lines(patch.target().path().get(), head).orElseThrow(() ->
new IllegalArgumentException("Could not get content for file " +
patch.target().path().get() +
" at revision " + head);
});
" at revision " + head)
);
}
stats = new WebrevStats(patch.asTextualPatch().stats(), newContent.size());
} else {
@@ -94,11 +99,11 @@ public ModifiedFileView(ReadOnlyRepository repo, Hash base, Hash head, List<Comm
if (head == null) {
binaryContent = Files.readAllBytes(repo.root().resolve(patch.target().path().get()));
} else {
binaryContent = repo.show(patch.target().path().get(), head).orElseThrow(() -> {
throw new IllegalArgumentException("Could not get content for file " +
binaryContent = repo.show(patch.target().path().get(), head).orElseThrow(() ->
new IllegalArgumentException("Could not get content for file " +
patch.target().path().get() +
" at revision " + head);
});
" at revision " + head)
);
}
stats = WebrevStats.empty();
}
@@ -29,6 +29,8 @@
import java.nio.channels.FileChannel;
import java.nio.file.*;
import java.util.*;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import java.util.function.Function;

import static java.nio.file.StandardOpenOption.*;
@@ -42,6 +44,8 @@ public class Webrev {

private static final String INDEX = "index.html";

private static final Logger log = Logger.getLogger("org.openjdk.skara.webrev");

public static final Set<String> STATIC_FILES =
Set.of(ANCNAV_HTML, ANCNAV_JS, ICON, CSS, INDEX);

@@ -198,6 +202,11 @@ private void generate(Diff diff, Hash tailEnd, Hash head) throws IOException {
}

var headHash = head == null ? repository.head() : head;
var filesDesc = files.isEmpty() ? "" :
" for files " +
files.stream().map(Path::toString).collect(Collectors.joining(", "));
log.fine("Generating webrev from " + tailEnd + " to " + headHash + filesDesc);

var fileViews = new ArrayList<FileView>();
var formatter = new MetadataFormatter(issueLinker);
for (var patch : patches) {