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

Switch to GitHub's new review comment positioning API #269

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
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
@@ -373,13 +373,16 @@ void addReviewComment(ReviewComment reviewComment) {

// Add some context to the first post
if (reviewComment.parent().isEmpty()) {
var contents = prInstance.pr().repository().fileContents(reviewComment.path(), reviewComment.hash().hex()).lines().collect(Collectors.toList());

body.append(reviewComment.path()).append(" line ").append(reviewComment.line()).append(":\n\n");
for (int i = Math.max(0, reviewComment.line() - 2); i < Math.min(contents.size(), reviewComment.line() + 1); ++i) {
body.append("> ").append(i + 1).append(": ").append(contents.get(i)).append("\n");
try {
var contents = prInstance.pr().repository().fileContents(reviewComment.path(), reviewComment.hash().hex()).lines().collect(Collectors.toList());
for (int i = Math.max(0, reviewComment.line() - 2); i < Math.min(contents.size(), reviewComment.line() + 1); ++i) {
body.append("> ").append(i + 1).append(": ").append(contents.get(i)).append("\n");
}
body.append("\n");
} catch (RuntimeException e) {
body.append("> (failed to retrieve contents of file, check the PR for context)\n");
}
body.append("\n");
}
body.append(reviewComment.body());

@@ -29,7 +29,6 @@
import org.openjdk.skara.network.*;
import org.openjdk.skara.vcs.Hash;

import java.io.*;
import java.net.URI;
import java.time.*;
import java.time.format.DateTimeFormatter;
@@ -119,14 +118,35 @@ public void addReview(Review.Verdict verdict, String body) {
.execute();
}

private ReviewComment parseReviewComment(ReviewComment parent, JSONObject json, PositionMapper diff) {
private ReviewComment parseReviewComment(ReviewComment parent, JSONObject json) {
var author = host.parseUserField(json);
var threadId = parent == null ? json.get("id").toString() : parent.threadId();

int line = json.get("original_line").asInt();
var hash = new Hash(json.get("original_commit_id").asString());
var path = json.get("path").asString();

if (json.get("side").asString().equals("LEFT")) {
var commitInfo = request.get("commits/" + hash).execute();

// It's possible that the file in question was renamed / deleted in an earlier commit, would
// need to parse all the commits in the PR to be sure. But this should cover most cases.
hash = new Hash(commitInfo.get("parents").asArray().get(0).get("sha").asString());
for (var file : commitInfo.get("files").asArray()) {
if (file.get("filename").asString().equals(path)) {
if (file.get("status").asString().equals("renamed")) {
path = file.get("previous_filename").asString();
}
break;
}
}
}

var comment = new ReviewComment(parent,
threadId,
new Hash(json.get("commit_id").asString()),
json.get("path").asString(),
diff.positionToLine(json.get("path").asString(), json.get("original_position").asInt()),
hash,
path,
line,
json.get("id").toString(),
json.get("body").asString(),
author,
@@ -137,74 +157,48 @@ private ReviewComment parseReviewComment(ReviewComment parent, JSONObject json,

@Override
public ReviewComment addReviewComment(Hash base, Hash hash, String path, int line, String body) {
try {
var rawDiff = request.get("pulls/" + json.get("number").toString())
.header("Accept", "application/vnd.github.v3.diff")
.executeUnparsed();
var diff = PositionMapper.parse(rawDiff);

var query = JSON.object()
.put("body", body)
.put("commit_id", hash.hex())
.put("path", path)
.put("position", diff.lineToPosition(path, line));
var response = request.post("pulls/" + json.get("number").toString() + "/comments")
.body(query)
.execute();
return parseReviewComment(null, response.asObject(), diff);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
var query = JSON.object()
.put("body", body)
.put("commit_id", hash.hex())
.put("path", path)
.put("side", "RIGHT")
.put("line", line);
var response = request.post("pulls/" + json.get("number").toString() + "/comments")
.body(query)
.execute();
return parseReviewComment(null, response.asObject());
}

@Override
public ReviewComment addReviewCommentReply(ReviewComment parent, String body) {
try {
var rawDiff = request.get("pulls/" + json.get("number").toString())
.header("Accept", "application/vnd.github.v3.diff")
.executeUnparsed();
var diff = PositionMapper.parse(rawDiff);

var query = JSON.object()
.put("body", body)
.put("in_reply_to", Integer.parseInt(parent.threadId()));
var response = request.post("pulls/" + json.get("number").toString() + "/comments")
.body(query)
.execute();
return parseReviewComment(parent, response.asObject(), diff);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
var query = JSON.object()
.put("body", body)
.put("in_reply_to", Integer.parseInt(parent.threadId()));
var response = request.post("pulls/" + json.get("number").toString() + "/comments")
.body(query)
.execute();
return parseReviewComment(parent, response.asObject());
}

@Override
public List<ReviewComment> reviewComments() {
try {
var rawDiff = request.get("pulls/" + json.get("number").toString())
.header("Accept", "application/vnd.github.v3.diff")
.executeUnparsed();
var diff = PositionMapper.parse(rawDiff);

var ret = new ArrayList<ReviewComment>();
var reviewComments = request.get("pulls/" + json.get("number").toString() + "/comments").execute().stream()
.map(JSONValue::asObject)
.collect(Collectors.toList());
var idToComment = new HashMap<String, ReviewComment>();

for (var reviewComment : reviewComments) {
ReviewComment parent = null;
if (reviewComment.contains("in_reply_to_id")) {
parent = idToComment.get(reviewComment.get("in_reply_to_id").toString());
}
var comment = parseReviewComment(parent, reviewComment, diff);
idToComment.put(comment.id(), comment);
ret.add(comment);
}
var ret = new ArrayList<ReviewComment>();
var reviewComments = request.get("pulls/" + json.get("number").toString() + "/comments").execute().stream()
.map(JSONValue::asObject)
.collect(Collectors.toList());
var idToComment = new HashMap<String, ReviewComment>();

return ret;
} catch (IOException e) {
throw new UncheckedIOException(e);
for (var reviewComment : reviewComments) {
ReviewComment parent = null;
if (reviewComment.contains("in_reply_to_id")) {
parent = idToComment.get(reviewComment.get("in_reply_to_id").toString());
}
var comment = parseReviewComment(parent, reviewComment);
idToComment.put(comment.id(), comment);
ret.add(comment);
}

return ret;
}

@Override
@@ -53,7 +53,8 @@ public class GitHubRepository implements HostedRepository {
"Authorization", "token " + gitHubHost.getInstallationToken(),
"Accept", "application/vnd.github.machine-man-preview+json",
"Accept", "application/vnd.github.antiope-preview+json",
"Accept", "application/vnd.github.shadow-cat-preview+json"));
"Accept", "application/vnd.github.shadow-cat-preview+json",
"Accept", "application/vnd.github.comfort-fade-preview+json"));
json = gitHubHost.getProjectInfo(repository);
var urlPattern = gitHubHost.getWebURI("/" + repository + "/pull/").toString();
pullRequestPattern = Pattern.compile(urlPattern + "(\\d+)");

This file was deleted.