From e54dc76fcb9d408e73bfa72950eaacef8ff890b6 Mon Sep 17 00:00:00 2001 From: Harjot Gill Date: Mon, 17 Apr 2023 19:19:05 -0700 Subject: [PATCH] Revert "create review instead of single comments" (#187) --- dist/index.js | 76 ++++++++++++++++++++++++--------------- src/commenter.ts | 93 ++++++++++++++++++++++++++++++------------------ src/review.ts | 10 ++---- 3 files changed, 109 insertions(+), 70 deletions(-) diff --git a/dist/index.js b/dist/index.js index 2f17b6a7..b9932f8d 100644 --- a/dist/index.js +++ b/dist/index.js @@ -3623,42 +3623,62 @@ ${tag}`; _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to get PR: ${e}, skipping adding release notes to description.`); } } - reviewCommentsBuffer = []; - async buffer_review_comment(path, start_line, end_line, message, tag = COMMENT_TAG) { + async review_comment(pull_number, commit_id, path, start_line, end_line, message, tag = COMMENT_TAG) { message = `${COMMENT_GREETING} ${message} ${tag}`; - this.reviewCommentsBuffer.push({ - path, - start_line, - end_line, - message - }); - } - async submit_review(pull_number, commit_id) { + // replace comment made by this action try { - if (this.reviewCommentsBuffer.length > 0) { - await octokit.pulls.createReview({ - owner: repo.owner, - repo: repo.repo, - pull_number, - commit_id, - event: 'COMMENT', - comments: this.reviewCommentsBuffer.map(comment => ({ - path: comment.path, - body: comment.message, - line: comment.end_line, - start_line: comment.start_line, + let found = false; + const comments = await this.get_comments_at_range(pull_number, path, start_line, end_line); + for (const comment of comments) { + if (comment.body.includes(tag)) { + _actions_core__WEBPACK_IMPORTED_MODULE_0__.info(`Updating review comment for ${path}:${start_line}-${end_line}: ${message}`); + await octokit.pulls.updateReviewComment({ + owner: repo.owner, + repo: repo.repo, + comment_id: comment.id, + body: message + }); + found = true; + break; + } + } + if (!found) { + _actions_core__WEBPACK_IMPORTED_MODULE_0__.info(`Creating new review comment for ${path}:${start_line}-${end_line}: ${message}`); + // if start_line is same as end_line, it's a single line comment + // otherwise it's a multi-line comment + if (start_line === end_line) { + await octokit.pulls.createReviewComment({ + owner: repo.owner, + repo: repo.repo, + pull_number, + body: message, + commit_id, + path, + line: end_line + }); + } + else { + await octokit.pulls.createReviewComment({ + owner: repo.owner, + repo: repo.repo, + pull_number, + body: message, + commit_id, + path, + line: end_line, + start_line, start_side: 'RIGHT' - })) - }); - this.reviewCommentsBuffer = []; + }); + } } } catch (e) { - _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to submit review: ${e}`); + _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to post review comment, for ${path}:${start_line}-${end_line}: ${e}`); + // throw error throw e; } } @@ -6822,7 +6842,7 @@ ${comment_chain} continue; } try { - await commenter.buffer_review_comment(filename, review.start_line, review.end_line, `${review.comment}`); + await commenter.review_comment(context.payload.pull_request.number, commits[commits.length - 1].sha, filename, review.start_line, review.end_line, `${review.comment}`); } catch (e) { reviews_failed.push(`${filename} comment failed (${e})`); @@ -6915,8 +6935,6 @@ ${reviews_failed.length > 0 } // post the final summary comment await commenter.comment(`${summarize_comment}`, lib_commenter/* SUMMARIZE_TAG */.Rp, 'replace'); - // post the review - await commenter.submit_review(context.payload.pull_request.number, commits[commits.length - 1].sha); }; const split_patch = (patch) => { if (!patch) { diff --git a/src/commenter.ts b/src/commenter.ts index eb51df7a..e4ea3713 100644 --- a/src/commenter.ts +++ b/src/commenter.ts @@ -129,14 +129,9 @@ ${tag}` } } - private reviewCommentsBuffer: { - path: string - start_line: number - end_line: number - message: string - }[] = [] - - async buffer_review_comment( + async review_comment( + pull_number: number, + commit_id: string, path: string, start_line: number, end_line: number, @@ -148,36 +143,66 @@ ${tag}` ${message} ${tag}` - - this.reviewCommentsBuffer.push({ - path, - start_line, - end_line, - message - }) - } - - async submit_review(pull_number: number, commit_id: string) { + // replace comment made by this action try { - if (this.reviewCommentsBuffer.length > 0) { - await octokit.pulls.createReview({ - owner: repo.owner, - repo: repo.repo, - pull_number, - commit_id, - event: 'COMMENT', - comments: this.reviewCommentsBuffer.map(comment => ({ - path: comment.path, - body: comment.message, - line: comment.end_line, - start_line: comment.start_line, + let found = false + const comments = await this.get_comments_at_range( + pull_number, + path, + start_line, + end_line + ) + for (const comment of comments) { + if (comment.body.includes(tag)) { + core.info( + `Updating review comment for ${path}:${start_line}-${end_line}: ${message}` + ) + await octokit.pulls.updateReviewComment({ + owner: repo.owner, + repo: repo.repo, + comment_id: comment.id, + body: message + }) + found = true + break + } + } + + if (!found) { + core.info( + `Creating new review comment for ${path}:${start_line}-${end_line}: ${message}` + ) + // if start_line is same as end_line, it's a single line comment + // otherwise it's a multi-line comment + if (start_line === end_line) { + await octokit.pulls.createReviewComment({ + owner: repo.owner, + repo: repo.repo, + pull_number, + body: message, + commit_id, + path, + line: end_line + }) + } else { + await octokit.pulls.createReviewComment({ + owner: repo.owner, + repo: repo.repo, + pull_number, + body: message, + commit_id, + path, + line: end_line, + start_line, start_side: 'RIGHT' - })) - }) - this.reviewCommentsBuffer = [] + }) + } } } catch (e) { - core.warning(`Failed to submit review: ${e}`) + core.warning( + `Failed to post review comment, for ${path}:${start_line}-${end_line}: ${e}` + ) + // throw error throw e } } diff --git a/src/review.ts b/src/review.ts index 2ead365e..2f7e2836 100644 --- a/src/review.ts +++ b/src/review.ts @@ -610,7 +610,9 @@ ${comment_chain} continue } try { - await commenter.buffer_review_comment( + await commenter.review_comment( + context.payload.pull_request.number, + commits[commits.length - 1].sha, filename, review.start_line, review.end_line, @@ -750,12 +752,6 @@ ${ // post the final summary comment await commenter.comment(`${summarize_comment}`, SUMMARIZE_TAG, 'replace') - - // post the review - await commenter.submit_review( - context.payload.pull_request.number, - commits[commits.length - 1].sha - ) } const split_patch = (patch: string | null | undefined): string[] => {