Skip to content
This repository was archived by the owner on Jul 1, 2024. It is now read-only.

Commit 6cc8d90

Browse files
committed
Apply suggestions from code review
1 parent 607c742 commit 6cc8d90

File tree

3 files changed

+21
-22
lines changed

3 files changed

+21
-22
lines changed

src/comments.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,4 +126,4 @@ I'll bump it to the DT maintainer queue. Thank you for your patience, @${author}
126126
// Introduction to the review when config files diverge from the
127127
// expected form
128128
export const suggestions = (user: string) =>
129-
`@${user} I noticed these differences from the expected form. If you can revise your changes to avoid them, so much the better! Otherwise please reply with explanations why they're needed (unless it's obvious) and a maintainer will take a look. Thanks!`;
129+
`@${user} I noticed these differences from the expected form. If you can revise your changes to avoid them, so much the better! Otherwise please reply with explanations why they're needed and a maintainer will take a look. Thanks!`;

src/execute-pr-actions.ts

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,17 @@ function getMutationsForCommentRemovals(actions: Actions, botComments: ParsedCom
103103
}
104104

105105
function getMutationsForSuggestions(actions: Actions, pr: PR_repository_pullRequest) {
106+
// Suggestions will be empty if we already reviewed this head
106107
if (actions.suggestions.length === 0) return [];
107-
if (!pr.author) throw new Error("Internal Error: no author is a bot error");
108+
if (!pr.author) throw new Error("Internal Error: expected to be checked");
108109
return [
109-
...actions.suggestions.map(({ path, startLine, endLine, body }) =>
110+
...actions.suggestions.map(({ path, startLine, endLine, text }) =>
110111
createMutation<schema.AddPullRequestReviewThreadInput>("addPullRequestReviewThread", {
111112
pullRequestId: pr.id,
112113
path,
113114
startLine: startLine === endLine ? undefined : startLine,
114115
line: endLine,
115-
body: "```suggestion\n" + body,
116+
body: "```suggestion\n" + text + "```",
116117
})
117118
),
118119
createMutation<schema.SubmitPullRequestReviewInput>("submitPullRequestReview", {
@@ -123,20 +124,17 @@ function getMutationsForSuggestions(actions: Actions, pr: PR_repository_pullRequ
123124
];
124125
}
125126

126-
function getMutationsForChangingPRState(actions: Actions, pr: PR_repository_pullRequest) {
127-
return [
128-
actions.shouldMerge
129-
? createMutation<schema.MergePullRequestInput>("mergePullRequest", {
130-
commitHeadline: `🤖 Merge PR #${pr.number} ${pr.title} by @${pr.author?.login ?? "(ghost)"}`,
131-
expectedHeadOid: pr.headRefOid,
132-
mergeMethod: "SQUASH",
133-
pullRequestId: pr.id,
134-
})
135-
: null,
136-
actions.shouldClose
137-
? createMutation<schema.ClosePullRequestInput>("closePullRequest", { pullRequestId: pr.id })
138-
: null
139-
];
127+
function* getMutationsForChangingPRState(actions: Actions, pr: PR_repository_pullRequest) {
128+
if (actions.shouldMerge) {
129+
if (!pr.author) throw new Error("Internal Error: expected to be checked");
130+
yield createMutation<schema.MergePullRequestInput>("mergePullRequest", {
131+
commitHeadline: `🤖 Merge PR #${pr.number} ${pr.title} by @${pr.author.login}`,
132+
expectedHeadOid: pr.headRefOid,
133+
mergeMethod: "SQUASH",
134+
pullRequestId: pr.id,
135+
});
136+
}
137+
if (actions.shouldClose) yield createMutation<schema.ClosePullRequestInput>("closePullRequest", { pullRequestId: pr.id });
140138
}
141139

142140
async function getProjectBoardColumnIdByName(name: string): Promise<string> {

src/pr-info.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ export type FileInfo = {
6666
export interface Suggestion {
6767
readonly startLine: number;
6868
readonly endLine: number;
69-
readonly body: string;
69+
readonly text: string;
7070
}
7171

7272
export type ReviewInfo = {
@@ -221,6 +221,7 @@ export async function deriveStateForPR(
221221
const refs = {
222222
head: prInfo.headRefOid,
223223
master: "master",
224+
// Exclude existing suggestions from subsequent reviews
224225
latestSuggestions: max(noNullish(prInfo.reviews?.nodes).filter(review => !authorNotBot(review)), (a, b) =>
225226
Date.parse(a.submittedAt) - Date.parse(b.submittedAt))?.commit?.oid,
226227
} as const;
@@ -436,8 +437,8 @@ function makeChecker(expectedForm: any, expectedFormUrl: string, options?: { par
436437
const vsMaster = await ignoreExistingDiffs("master");
437438
if (!vsMaster) return undefined;
438439
if (vsMaster.done) return { suspect: vsMaster.suspect };
439-
// whereas getting closer relative to existing suggestions makes
440-
// no further suggestions
440+
// whereas getting closer relative to existing suggestions means
441+
// no new suggestions
441442
if (!await ignoreExistingDiffs("latestSuggestions")) return { suspect: vsMaster.suspect };
442443
jsonDiff.applyPatch(suggestion, jsonDiff.compare(newData, towardsIt));
443444
return {
@@ -494,7 +495,7 @@ function makeChecker(expectedForm: any, expectedFormUrl: string, options?: { par
494495
return {
495496
startLine,
496497
endLine,
497-
body: suggestionLines.join(""),
498+
text: suggestionLines.join(""),
498499
};
499500
}
500501
};

0 commit comments

Comments
 (0)