Public: anyone can read this; the crew can change it.

A GitHub migration puts each carried issue and pull in its milestone #4018

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/3830-a-github-migration-puts-each-carried-iss → main 2 commits fork

  • Merged. scout/3830-a-github-migration-puts-each-carried-iss was merged into main as dc58620.

Review

Sign in to review this change.
6 files changed +68 −4
src/origins/carry.ts +10 −2
@@ -26,7 +26,7 @@
2626 */
2727
2828 import type { Database } from "bun:sqlite";
29 import { carryMilestone } from "./milestones.ts";
29 import { carryMilestone, joinMilestones } from "./milestones.ts";
3030 import { importComments, importIssue, importLabels } from "./import.ts";
3131 import type { ListIssuesQuery, Provider, RemoteIssue } from "./types.ts";
3232
@@ -97,6 +97,7 @@ export async function carryConversations(input: CarryInput): Promise<Carried> {
9797 let error = "";
9898 const counts = { labels: 0, issues: 0, pulls: 0, comments: 0, milestones: 0 };
9999 const forkPulls: ForkPull[] = [];
100 const members: { issueId: string; milestone: number }[] = [];
100101
101102 const labels = await provider.listLabels(fullName);
102103 if (labels.ok) {
@@ -137,7 +138,13 @@ export async function carryConversations(input: CarryInput): Promise<Carried> {
137138 ).run(imported.issueId);
138139 else forkPulls.push({ issueId: imported.issueId, number: remote.number });
139140 counts.pulls++;
140 } else counts.issues++;
141 } else {
142 counts.issues++;
143 // A pull is not an issue to the plan (`core/dependencies.ts`), so
144 // only issues can wait inside a milestone.
145 if (remote.milestone !== undefined)
146 members.push({ issueId: imported.issueId, milestone: remote.milestone });
147 }
141148
142149 if (remote.comments > 0) {
143150 const comments = await provider.listComments(fullName, remote.number);
@@ -167,6 +174,7 @@ export async function carryConversations(input: CarryInput): Promise<Carried> {
167174 counts.milestones++;
168175 } else error ||= listed.error.message;
169176 }
177 joinMilestones(db, repoId, actorId, members);
170178 return { ...counts, forkPulls, error };
171179 }
172180
src/origins/milestones.ts +30 −2
@@ -8,11 +8,19 @@
88 * which is also how a second carry finds the milestone it made the first
99 * time and leaves it alone — what a person has changed since is theirs.
1010 *
11 * Which issues belong to which milestone is not carried here.
11 * Which issues belong to which milestone is `joinMilestones`: the milestone
12 * waits for each member, and linking twice leaves one link.
1213 */
1314
1415 import type { Database } from "bun:sqlite";
15 import { createIssue, MAX_BODY, MAX_TITLE, setIssueState } from "../core/issues.ts";
16 import { addDependency } from "../core/dependencies.ts";
17 import {
18 createIssue,
19 getIssueById,
20 MAX_BODY,
21 MAX_TITLE,
22 setIssueState,
23 } from "../core/issues.ts";
1624 import type { RemoteMilestone } from "./types.ts";
1725
1826 /** Control bytes out, length capped: an origin does not get to write a novel. */
@@ -66,3 +74,23 @@ export function carryMilestone(
6674 ).run(repoId, remote.number, made.value.id, remote.dueOn);
6775 return { issueId: made.value.id, created: true };
6876 }
77
78 /** Put each carried issue in the milestone the origin named, if that one was carried. */
79 export function joinMilestones(
80 db: Database,
81 repoId: string,
82 actorId: string,
83 members: readonly { readonly issueId: string; readonly milestone: number }[],
84 ): void {
85 const find = db.query<{ issue_id: string }, [string, number]>(
86 "SELECT issue_id FROM milestone_origin WHERE repo_id = ?1 AND origin_number = ?2",
87 );
88 for (const member of members) {
89 const home = find.get(repoId, member.milestone);
90 if (home === null) continue;
91 const milestone = getIssueById(db, home.issue_id);
92 const issue = getIssueById(db, member.issueId);
93 if (milestone !== undefined && issue !== undefined)
94 addDependency(db, milestone, issue, actorId);
95 }
96 }
src/origins/providers/github.ts +2
@@ -784,7 +784,9 @@ function toReviewState(raw: unknown): RemoteReviewState | undefined {
784784 }
785785
786786 function toIssue(raw: unknown): RemoteIssue {
787 const milestone = asNumber(dig(raw, "milestone", "number"));
787788 return {
789 ...(milestone > 0 ? { milestone } : {}),
788790 remoteId: asId(field(raw, "id")),
789791 number: asNumber(field(raw, "number")),
790792 title: asString(field(raw, "title")),
src/origins/types.ts +2
@@ -122,6 +122,8 @@ export interface RemoteIssue {
122122 readonly closedAt: string | null;
123123 readonly htmlUrl: string;
124124 readonly comments: number;
125 /** The origin's number of the milestone it belongs to, when it does. */
126 readonly milestone?: number;
125127 /**
126128 * `owner/name` on the origin. Only a cross-repo result (search) carries
127129 * it — a per-repo list already knows which repo it asked about.
test/fake-origin.ts +5
@@ -70,6 +70,8 @@ export interface FakeIssue {
7070 closedAt?: string | null;
7171 /** What the listing says `comments` is; the thread itself is `comments`. */
7272 commentCount?: number;
73 /** GitHub: the number of the milestone it belongs to. */
74 milestone?: number;
7375 /** Present for a pull request. */
7476 pull?: {
7577 base: string;
@@ -1014,6 +1016,9 @@ function issueJson(
10141016 closed_at: issue.closedAt ?? null,
10151017 html_url: `${url.origin}/${issue.number}`,
10161018 comments: issue.commentCount ?? 0,
1019 ...(issue.milestone === undefined
1020 ? {}
1021 : { milestone: { number: issue.milestone } }),
10171022 };
10181023 if (issue.pull === undefined) return common;
10191024 // GitHub marks a pull by the mere presence of this key; Forgejo puts the
test/migrate-milestones.test.ts +19
@@ -37,6 +37,9 @@ describe("migrate carries milestones", () => {
3737 });
3838 state.issues.set("octo/gadgets", [
3939 { id: 11, number: 1, title: "A conversation", user: "mona" },
40 { id: 12, number: 2, title: "In v1", user: "mona", milestone: 1 },
41 { id: 13, number: 3, title: "Also v1", user: "mona", milestone: 1 },
42 { id: 14, number: 4, title: "In v0", user: "mona", milestone: 2 },
4043 ]);
4144 state.milestones.set("octo/gadgets", [
4245 { number: 1, title: "v1", description: "The first cut.", dueOn: "2026-12-01T00:00:00Z" },
@@ -87,6 +90,14 @@ describe("migrate carries milestones", () => {
8790 )
8891 .all(repo.value.id);
8992 expect(rows.map((r) => r.origin_number)).toEqual([1, 2, 3]);
93 const waits = () =>
94 app.db
95 .query<{ m: number; n: number }, []>(
96 `SELECT m.origin_number AS m, i.number AS n FROM issue_dependency d
97 JOIN milestone_origin m ON m.issue_id = d.issue_id
98 JOIN issue i ON i.id = d.blocked_by_id ORDER BY m, n`,
99 )
100 .all();
90101 const issues = rows.map((r) => {
91102 const issue = app.db
92103 .query<{ title: string; body: string; state: string }, [string]>(
@@ -100,6 +111,13 @@ describe("migrate carries milestones", () => {
100111 { title: "v0", body: "Done long ago.", state: "closed" },
101112 { title: "Someday", body: "", state: "open" },
102113 ]);
114 // Issues 2 and 3 landed in v1, 4 in v0; the unfiled one in none.
115 const members = waits().map((w) => [w.m, getIssue(app.db, repo.value.id, w.n)?.title]);
116 expect(members).toEqual([
117 [1, "In v1"],
118 [1, "Also v1"],
119 [2, "In v0"],
120 ]);
103121 expect(dueOf(app.db, repo.value.id, 1)).toBe("2026-12-01T00:00:00.000Z");
104122 expect(dueOf(app.db, repo.value.id, 2)).toBe("2026-01-15T00:00:00.000Z");
105123 expect(dueOf(app.db, repo.value.id, 3)).toBeNull();
@@ -109,6 +127,7 @@ describe("migrate carries milestones", () => {
109127 const before = app.db.query<{ n: number }, []>("SELECT count(*) AS n FROM issue").get()!.n;
110128 const second = await carry();
111129 expect(second.milestones).toBe(0);
130 expect(waits().length).toBe(3);
112131 expect(app.db.query<{ n: number }, []>("SELECT count(*) AS n FROM issue").get()!.n).toBe(before);
113132 expect(
114133 app.db.query<{ n: number }, []>("SELECT count(*) AS n FROM milestone_origin").get()!.n,

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs