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

Commit f050a2e

Browse files
committed
Apply suggestions from code review
1 parent 9f2ffa3 commit f050a2e

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
@@ -64,7 +64,7 @@ export type FileInfo = {
6464
export interface Suggestion {
6565
readonly startLine: number;
6666
readonly endLine: number;
67-
readonly body: string;
67+
readonly text: string;
6868
}
6969

7070
export type ReviewInfo = {
@@ -219,6 +219,7 @@ export async function deriveStateForPR(
219219
const refs = {
220220
head: prInfo.headRefOid,
221221
master: "master",
222+
// Exclude existing suggestions from subsequent reviews
222223
latestSuggestions: max(noNullish(prInfo.reviews?.nodes).filter(review => !authorNotBot(review)), (a, b) =>
223224
Date.parse(a.submittedAt) - Date.parse(b.submittedAt))?.commit?.oid,
224225
} as const;
@@ -434,8 +435,8 @@ function makeChecker(expectedForm: any, expectedFormUrl: string, options?: { par
434435
const vsMaster = await ignoreExistingDiffs("master");
435436
if (!vsMaster) return undefined;
436437
if (vsMaster.done) return { suspect: vsMaster.suspect };
437-
// whereas getting closer relative to existing suggestions makes
438-
// no further suggestions
438+
// whereas getting closer relative to existing suggestions means
439+
// no new suggestions
439440
if (!await ignoreExistingDiffs("latestSuggestions")) return { suspect: vsMaster.suspect };
440441
jsonDiff.applyPatch(suggestion, jsonDiff.compare(newData, towardsIt));
441442
return {
@@ -492,7 +493,7 @@ function makeChecker(expectedForm: any, expectedFormUrl: string, options?: { par
492493
return {
493494
startLine,
494495
endLine,
495-
body: suggestionLines.join(""),
496+
text: suggestionLines.join(""),
496497
};
497498
}
498499
};

0 commit comments

Comments
 (0)