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

Commit 3602444

Browse files
committed
Apply suggestions from code review
1 parent 47f96e8 commit 3602444

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
@@ -63,7 +63,7 @@ export type FileInfo = {
6363
export interface Suggestion {
6464
readonly startLine: number;
6565
readonly endLine: number;
66-
readonly body: string;
66+
readonly text: string;
6767
}
6868

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

0 commit comments

Comments
 (0)