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

Commit 300c39d

Browse files
committed
Apply suggestions from code review
1 parent bdcbf52 commit 300c39d

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
@@ -112,4 +112,4 @@ I'll bump it to the DT maintainer queue. Thank you for your patience, @${author}
112112
// Introduction to the review when config files diverge from the
113113
// required form
114114
export const Suggestions = (user: string) =>
115-
`@${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!`;
115+
`@${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
@@ -111,17 +111,18 @@ function getMutationsForCommentRemovals(actions: Actions, botComments: ParsedCom
111111
}
112112

113113
function getMutationsForSuggestions(actions: Actions, pr: PR_repository_pullRequest) {
114+
// Suggestions will be empty if we already reviewed this head
114115
if (actions.suggestions.length === 0) return [];
115-
if (!pr.author) throw new Error("Internal Error: no author is a bot error");
116+
if (!pr.author) throw new Error("Internal Error: expected to be checked");
116117
return [
117-
...actions.suggestions.map(({ path, startLine, endLine, body }) =>
118+
...actions.suggestions.map(({ path, startLine, endLine, text }) =>
118119
createMutation(addPullRequestReviewThread, {
119120
input: {
120121
pullRequestId: pr.id,
121122
path,
122123
startLine: startLine === endLine ? undefined : startLine,
123124
line: endLine,
124-
body: "```suggestion\n" + body,
125+
body: "```suggestion\n" + text + "```",
125126
}
126127
})
127128
),
@@ -135,22 +136,19 @@ function getMutationsForSuggestions(actions: Actions, pr: PR_repository_pullRequ
135136
];
136137
}
137138

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

156154
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
@@ -80,7 +80,7 @@ export type FileInfo = {
8080
export interface Suggestion {
8181
readonly startLine: number;
8282
readonly endLine: number;
83-
readonly body: string;
83+
readonly text: string;
8484
}
8585

8686
export type ReviewInfo = {
@@ -235,6 +235,7 @@ export async function deriveStateForPR(
235235
const refs = {
236236
head: headCommit.oid,
237237
master: "master",
238+
// Exclude existing suggestions from subsequent reviews
238239
latestSuggestions: prInfo.reviews?.nodes?.reduce((latest, review) =>
239240
review && !authorNotBot(review) && (
240241
!latest?.submittedAt || review.submittedAt && new Date (review.submittedAt) > new Date(latest.submittedAt))
@@ -448,8 +449,8 @@ function makeJsonCheckerFromCore(requiredForm: any, ignoredKeys: string[]) {
448449
const vsMaster = await ignoreExistingDiffs("master");
449450
if (!vsMaster) return undefined;
450451
if (vsMaster.done) return { suspect: vsMaster.suspect };
451-
// whereas getting closer relative to existing suggestions makes
452-
// no further suggestions
452+
// whereas getting closer relative to existing suggestions means
453+
// no new suggestions
453454
if (!await ignoreExistingDiffs("latestSuggestions")) return { suspect: vsMaster.suspect };
454455
jsonDiff.applyPatch(suggestion, jsonDiff.compare(newJson, towardsIt));
455456
return {
@@ -501,7 +502,7 @@ function makeJsonCheckerFromCore(requiredForm: any, ignoredKeys: string[]) {
501502
return {
502503
startLine,
503504
endLine,
504-
body: suggestionLines.join(""),
505+
text: suggestionLines.join(""),
505506
};
506507
}
507508
};

0 commit comments

Comments
 (0)