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

Commit eb27ddb

Browse files
committed
Apply suggestions from code review
1 parent cc50598 commit eb27ddb

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
@@ -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
// required form
128128
export const Suggestions = (user: string) =>
129-
`@${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!`;
129+
`@${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
@@ -78,7 +78,7 @@ export type FileInfo = {
7878
export interface Suggestion {
7979
readonly startLine: number;
8080
readonly endLine: number;
81-
readonly body: string;
81+
readonly text: string;
8282
}
8383

8484
export type ReviewInfo = {
@@ -242,6 +242,7 @@ export async function deriveStateForPR(
242242
const refs = {
243243
head: headCommit.oid,
244244
master: "master",
245+
// Exclude existing suggestions from subsequent reviews
245246
latestSuggestions: prInfo.reviews?.nodes?.reduce((latest, review) =>
246247
review && !authorNotBot(review) && (
247248
!latest?.submittedAt || review.submittedAt && new Date (review.submittedAt) > new Date(latest.submittedAt))
@@ -455,8 +456,8 @@ function makeJsonCheckerFromCore(requiredForm: any, ignoredKeys: string[], requi
455456
const vsMaster = await ignoreExistingDiffs("master");
456457
if (!vsMaster) return undefined;
457458
if (vsMaster.done) return { suspect: vsMaster.suspect };
458-
// whereas getting closer relative to existing suggestions makes
459-
// no further suggestions
459+
// whereas getting closer relative to existing suggestions means
460+
// no new suggestions
460461
if (!await ignoreExistingDiffs("latestSuggestions")) return { suspect: vsMaster.suspect };
461462
jsonDiff.applyPatch(suggestion, jsonDiff.compare(newJson, towardsIt));
462463
return {
@@ -511,7 +512,7 @@ function makeJsonCheckerFromCore(requiredForm: any, ignoredKeys: string[], requi
511512
return {
512513
startLine,
513514
endLine,
514-
body: suggestionLines.join(""),
515+
text: suggestionLines.join(""),
515516
};
516517
}
517518
};

0 commit comments

Comments
 (0)