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

Commit 07c6125

Browse files
committed
Apply suggestions from code review
1 parent 46c3328 commit 07c6125

File tree

3 files changed

+23
-24
lines changed

3 files changed

+23
-24
lines changed

src/comments.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,4 @@ I'll bump it to the DT maintainer queue. Thank you for your patience, @${author}
120120
// Introduction to the review when config files diverge from the
121121
// required form
122122
export const Suggestions = (user: string) =>
123-
`@${user} I noticed these differences from the required 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!`;
123+
`@${user} I noticed these differences from the required 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: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -114,17 +114,18 @@ function getMutationsForCommentRemovals(actions: Actions, botComments: ParsedCom
114114
}
115115

116116
function getMutationsForSuggestions(actions: Actions, pr: PR_repository_pullRequest) {
117+
// Suggestions will be empty if we already reviewed this head
117118
if (actions.suggestions.length === 0) return [];
118-
if (!pr.author) throw new Error("Internal Error: no author is a bot error");
119+
if (!pr.author) throw new Error("Internal Error: expected to be checked");
119120
return [
120-
...actions.suggestions.map(({ path, startLine, endLine, body }) =>
121+
...actions.suggestions.map(({ path, startLine, endLine, text }) =>
121122
createMutation(addPullRequestReviewThread, {
122123
input: {
123124
pullRequestId: pr.id,
124125
path,
125126
startLine: startLine === endLine ? undefined : startLine,
126127
line: endLine,
127-
body: "```suggestion\n" + body,
128+
body: "```suggestion\n" + text + "```",
128129
}
129130
})
130131
),
@@ -138,22 +139,19 @@ function getMutationsForSuggestions(actions: Actions, pr: PR_repository_pullRequ
138139
];
139140
}
140141

141-
function getMutationsForChangingPRState(actions: Actions, pr: PR_repository_pullRequest) {
142-
return [
143-
actions.shouldMerge
144-
? createMutation(mergePr, {
145-
input: {
146-
commitHeadline: `🤖 Merge PR #${pr.number} ${pr.title} by @${pr.author?.login ?? "(ghost)"}`,
147-
expectedHeadOid: pr.headRefOid,
148-
mergeMethod: "SQUASH",
149-
pullRequestId: pr.id,
150-
},
151-
})
152-
: null,
153-
actions.shouldClose
154-
? createMutation(closePr, { input: { pullRequestId: pr.id } })
155-
: null
156-
];
142+
function* getMutationsForChangingPRState(actions: Actions, pr: PR_repository_pullRequest) {
143+
if (actions.shouldMerge) {
144+
if (!pr.author) throw new Error("Internal Error: expected to be checked");
145+
yield createMutation(mergePr, {
146+
input: {
147+
commitHeadline: `🤖 Merge PR #${pr.number} ${pr.title} by @${pr.author.login}`,
148+
expectedHeadOid: pr.headRefOid,
149+
mergeMethod: "SQUASH",
150+
pullRequestId: pr.id,
151+
},
152+
});
153+
}
154+
if (actions.shouldClose) yield createMutation(closePr, { input: { pullRequestId: pr.id } });
157155
}
158156

159157
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
@@ -81,7 +81,7 @@ export type FileInfo = {
8181
export interface Suggestion {
8282
readonly startLine: number;
8383
readonly endLine: number;
84-
readonly body: string;
84+
readonly text: string;
8585
}
8686

8787
export type ReviewInfo = {
@@ -236,6 +236,7 @@ export async function deriveStateForPR(
236236
const refs = {
237237
head: headCommit.oid,
238238
master: "master",
239+
// Exclude existing suggestions from subsequent reviews
239240
latestSuggestions: prInfo.reviews?.nodes?.reduce((latest, review) =>
240241
review && !authorNotBot(review) && (
241242
!latest?.submittedAt || review.submittedAt && new Date (review.submittedAt) > new Date(latest.submittedAt))
@@ -454,8 +455,8 @@ function makeJsonCheckerFromCore(requiredForm: any, ignoredKeys: string[], requi
454455
const vsMaster = await ignoreExistingDiffs("master");
455456
if (!vsMaster) return undefined;
456457
if (vsMaster.done) return { suspect: vsMaster.suspect };
457-
// whereas getting closer relative to existing suggestions makes
458-
// no further suggestions
458+
// whereas getting closer relative to existing suggestions means
459+
// no new suggestions
459460
if (!await ignoreExistingDiffs("latestSuggestions")) return { suspect: vsMaster.suspect };
460461
jsonDiff.applyPatch(suggestion, jsonDiff.compare(newJson, towardsIt));
461462
return {
@@ -510,7 +511,7 @@ function makeJsonCheckerFromCore(requiredForm: any, ignoredKeys: string[], requi
510511
return {
511512
startLine,
512513
endLine,
513-
body: suggestionLines.join(""),
514+
text: suggestionLines.join(""),
514515
};
515516
}
516517
};

0 commit comments

Comments
 (0)