[Add] Work on a Gutenberg issue and open a pull request to it (#251)

src/github-pr.cjs

@@ -46,15 +46,34 @@ const UPSTREAM_REPO = 'wordpress-develop';
  * sandbox needs a `trunk` branch and must not be owned by the signed-in
  * account, since an account cannot fork its own repository.
  *
+ * The site's project type supplies the repository for everything that is not a
+ * sandbox run (#251), so a Gutenberg site forks and targets WordPress/gutenberg.
+ * The environment override still wins, because its whole purpose is to redirect
+ * a real run away from a real upstream.
+ *
+ * @param {Object} [project] The project type's `upstream` config.
  * @return {{owner: string, repo: string}}
  */
-function upstream() {
+function upstream(project) {
 	const raw = process.env.WP_DEV_ENV_GITHUB_UPSTREAM;
 	const match = typeof raw === 'string' && /^([^/\s]+)\/([^/\s]+)$/.exec(raw.trim());
 	if (match) return { owner: match[1], repo: match[2] };
+	if (project && project.owner && project.repo) return { owner: project.owner, repo: project.repo };
 	return { owner: UPSTREAM_OWNER, repo: UPSTREAM_REPO };
 }
 
+/**
+ * The branch a pull request targets. Both projects call it `trunk` today, but
+ * reading it from the project type rather than a constant is what keeps a third
+ * one from needing a second code path.
+ *
+ * @param {Object} [project] The project type's `upstream` config.
+ * @return {string}
+ */
+function baseBranchFor(project) {
+	return (project && project.base) || BASE_BRANCH;
+}
+
 /**
  * True when this run stops before opening the pull request.
  *
@@ -77,6 +96,10 @@ function isDryRun() {
  * @return {{dryRun: boolean, target: string}|null}
  */
 function testMode() {
+	// Deliberately without a project: this answers "is the environment
+	// redirecting a real run somewhere else", which is an env-override question.
+	// Reading a project's own upstream here would report every Gutenberg site as
+	// sandboxed simply for not being wordpress-develop.
 	const up = upstream();
 	const target = `${up.owner}/${up.repo}`;
 	const sandboxed = target !== `${UPSTREAM_OWNER}/${UPSTREAM_REPO}`;
@@ -176,16 +199,22 @@ const MAX_NOTES_LENGTH = 20000;
  * @param {number|string} root0.ticketId
  * @param {string}        [root0.handle]
  * @param {string}        [root0.event]
- * @param {string}        [root0.notes]  Free text from the contributor.
+ * @param {string}        [root0.notes]   Free text from the contributor.
+ * @param {Object}        [root0.project] The project type's `pr` config plus its work-item URL.
  * @return {string}
  */
-function buildPullRequestBody({ ticketId, handle, event, notes } = {}) {
+function buildPullRequestBody({ ticketId, handle, event, notes, project } = {}) {
 	const lines = [];
 
 	const written = typeof notes === 'string' ? notes.trim().slice(0, MAX_NOTES_LENGTH) : '';
 	if (written) lines.push(written, '');
 
-	lines.push(`Trac ticket: ${ticketUrl(ticketId)}`);
+	// How a pull request names its work item is the project's own convention
+	// (#251): Core cites the Trac URL, Gutenberg closes the issue with
+	// `Fixes #1234`. Defaults to Core's when no project is supplied.
+	lines.push(project && typeof project.bodyLine === 'function'
+		? project.bodyLine(ticketId, project.workItemUrl || ticketUrl(ticketId))
+		: `Trac ticket: ${ticketUrl(ticketId)}`);
 	// The same two facts the mentor-handoff header carries (#166), for the same
 	// reason: props follow whoever wrote the patch, and a contributor-day room
 	// is worth naming while it is still happening.
@@ -196,14 +225,19 @@ function buildPullRequestBody({ ticketId, handle, event, notes } = {}) {
 }
 
 /**
- * A branch name for a ticket, and the alternatives to try if it is taken.
+ * A branch name for a work item, and the alternatives to try if it is taken.
+ *
+ * The prefix comes from the project type (#251) — `trac-` for a Core ticket,
+ * `fix/issue-` for a Gutenberg issue — so the branch reads correctly in the
+ * repository it is pushed to.
  *
  * @param {number|string} ticketId
  * @param {number}        attempt  Zero for the first try.
+ * @param {string}        [prefix] Defaults to Core's.
  * @return {string}
  */
-function branchNameFor(ticketId, attempt = 0) {
-	const base = `trac-${String(ticketId).replace(/[^0-9]/g, '')}`;
+function branchNameFor(ticketId, attempt = 0, prefix = 'trac-') {
+	const base = `${prefix}${String(ticketId).replace(/[^0-9]/g, '')}`;
 	return attempt === 0 ? base : `${base}-${attempt + 1}`;
 }
 
@@ -224,7 +258,7 @@ async function ensureFork({ token, login }, deps = {}) {
 	const post = deps.post || postJson;
 	const wait = deps.sleep || sleep;
 	const attempts = deps.forkPollAttempts || FORK_POLL_ATTEMPTS;
-	const up = upstream();
+	const up = upstream(deps.project);
 	const forkUrl = `${API}/repos/${login}/${up.repo}`;
 
 	// A repository under the fork's name is only usable if it actually is a
@@ -247,7 +281,7 @@ async function ensureFork({ token, login }, deps = {}) {
 	// surfaces at the very last write — the branch — as an opaque 404. Found
 	// by hand on the first real run against this repository, which is big
 	// enough for that window to be minutes wide.
-	const readRefs = () => get(`${forkUrl}/git/ref/heads/${BASE_BRANCH}`, { token });
+	const readRefs = () => get(`${forkUrl}/git/ref/heads/${baseBranchFor(deps.project)}`, { token });
 
 	let existing;
 	try {
@@ -323,16 +357,16 @@ async function ensureFork({ token, login }, deps = {}) {
 async function resolveBase({ token, login, baseSha }, deps = {}) {
 	const get = deps.get || getJson;
 	const post = deps.post || postJson;
-	const repo = `${API}/repos/${login}/${upstream().repo}`;
+	const repo = `${API}/repos/${login}/${upstream(deps.project).repo}`;
 
 	try {
 		// Always fast-forward first, so "the tip" means today's trunk and not
 		// wherever the fork was left. 409 here is a diverged fork, which is a
 		// normal state for someone who has contributed before — not a failure
 		// to report; the branch then bases on the fork's own tip.
-		await post(`${repo}/merge-upstream`, { branch: BASE_BRANCH }, { token });
+		await post(`${repo}/merge-upstream`, { branch: baseBranchFor(deps.project) }, { token });
 
-		const ref = await get(`${repo}/git/ref/heads/${BASE_BRANCH}`, { token });
+		const ref = await get(`${repo}/git/ref/heads/${baseBranchFor(deps.project)}`, { token });
 		if (ref.status !== 200 || !ref.json || !ref.json.object || !ref.json.object.sha) {
 			return failure(ref, 'Could not read your fork’s trunk');
 		}
@@ -374,7 +408,7 @@ async function resolveBase({ token, login, baseSha }, deps = {}) {
  */
 async function staleTouchedPaths({ token, login, tipSha, files }, deps = {}) {
 	const get = deps.get || getJson;
-	const repo = `${API}/repos/${login}/${upstream().repo}`;
+	const repo = `${API}/repos/${login}/${upstream(deps.project).repo}`;
 
 	const clashes = [];
 	try {
@@ -418,7 +452,7 @@ async function staleTouchedPaths({ token, login, tipSha, files }, deps = {}) {
  */
 async function createTree({ token, login, baseTreeSha, files }, deps = {}) {
 	const post = deps.post || postJson;
-	const repo = `${API}/repos/${login}/${upstream().repo}`;
+	const repo = `${API}/repos/${login}/${upstream(deps.project).repo}`;
 	const entries = [];
 
 	try {
@@ -466,7 +500,7 @@ async function createTree({ token, login, baseTreeSha, files }, deps = {}) {
  */
 async function commitAndBranch({ token, login, ticketId, message, treeSha, parentSha }, deps = {}) {
 	const post = deps.post || postJson;
-	const repo = `${API}/repos/${login}/${upstream().repo}`;
+	const repo = `${API}/repos/${login}/${upstream(deps.project).repo}`;
 
 	try {
 		const commit = await post(`${repo}/git/commits`, {
@@ -479,7 +513,7 @@ async function commitAndBranch({ token, login, ticketId, message, treeSha, paren
 
 		let lastRes = null;
 		for (let attempt = 0; attempt < MAX_BRANCH_ATTEMPTS; attempt++) {
-			const branch = branchNameFor(ticketId, attempt);
+			const branch = branchNameFor(ticketId, attempt, deps.branchPrefix);
 			const ref = await post(`${repo}/git/refs`, { ref: `refs/heads/${branch}`, sha }, { token });
 			if (ref.status === 201) return { ok: true, branch, sha };
 			// A 404 here is the fork's ref database still initialising — the
@@ -520,12 +554,12 @@ async function commitAndBranch({ token, login, ticketId, message, treeSha, paren
 async function createPullRequest({ token, login, branch, title, body }, deps = {}) {
 	const post = deps.post || postJson;
 	try {
-		const up = upstream();
+		const up = upstream(deps.project);
 		const res = await post(`${API}/repos/${up.owner}/${up.repo}/pulls`, {
 			title,
 			body,
 			head: `${login}:${branch}`,
-			base: BASE_BRANCH,
+			base: baseBranchFor(deps.project),
 			maintainer_can_modify: true
 		}, { token });
 		if (res.status !== 201 || !res.json || !res.json.html_url) return failure(res, 'Could not open the pull request');
@@ -551,11 +585,19 @@ async function createPullRequest({ token, login, branch, title, body }, deps = {
  * @param {Array}         root0.files
  * @param {string}        root0.title
  * @param {string}        root0.body
+ * @param {Object}        [root0.project]    The project type's `upstream` + branch prefix.
  * @param {Function}      [root0.onProgress]
  * @param {Object}        [deps]
  * @return {Promise<{ok: true, url: string, number: number, branch: string, exactBase: boolean}|{ok: false, reason: string, error: string, stage: string}>}
  */
-async function openPullRequest({ token, login, ticketId, baseSha, files, title, body, onProgress }, deps = {}) {
+async function openPullRequest({ token, login, ticketId, baseSha, files, title, body, project, onProgress }, deps = {}) {
+	// The project type rides in `deps` so every helper below — fork, sync, tree,
+	// branch, pull request — targets the same repository and base branch without
+	// each one growing its own parameter (#251). Absent, they all default to
+	// wordpress-develop, which is what a site with no project type is.
+	if (project) {
+		deps = { ...deps, project: project.upstream, branchPrefix: project.branchPrefix };
+	}
 	const get = deps.get || getJson;
 	const report = typeof onProgress === 'function' ? onProgress : () => {};
 	const at = (stage, result) => ({ ...result, stage });
@@ -596,7 +638,7 @@ async function openPullRequest({ token, login, ticketId, baseSha, files, title,
 	// the wrong one silently produces a tree with no history behind it.
 	let baseCommit;
 	try {
-		baseCommit = await get(`${API}/repos/${login}/${upstream().repo}/git/commits/${base.sha}`, { token });
+		baseCommit = await get(`${API}/repos/${login}/${upstream(deps.project).repo}/git/commits/${base.sha}`, { token });
 	} catch (e) {
 		return at('syncing', { ok: false, reason: 'offline', error: String(e && e.message ? e.message : e) });
 	}
@@ -627,7 +669,7 @@ async function openPullRequest({ token, login, ticketId, baseSha, files, title,
 		return {
 			ok: true,
 			dryRun: true,
-			url: `https://github.com/${login}/${upstream().repo}/tree/${branched.branch}`,
+			url: `https://github.com/${login}/${upstream(deps.project).repo}/tree/${branched.branch}`,
 			number: null,
 			branch: branched.branch,
 			exactBase: base.exact

src/main.js

@@ -66,7 +66,7 @@ const SWITCH_PROGRESS_CHANNEL = 'switch:progress';
 // step of an operation, and describing it as progress would have the panel say
 // "Saving your work…" about trunk — which is the one thing this refuses to do.
 const CARRIED_WORK_CHANNEL = 'ticket:carried-work';
-const { parseTicketRef } = require('./renderer/trac-ticket.cjs');
+const { workItemProvider } = require('./work-item.cjs');
 const { parseHandle } = require('./wporg-handle.cjs');
 const { parseEventName, buildProvenanceHeader, handoffFilename } = require('./patch-provenance.cjs');
 const { describeRefused } = require('./safe-log');
@@ -619,10 +619,16 @@ ipcMain.handle('git:save-patch', async (_e, sitePath, options) => {
             const s = await getStore();
             const meta = (s.get('siteMeta') || {})[sitePath] || {};
             const { wporgHandle: handle = null, contributionEvent: event = null } = s.get('preferences') || {};
+            // The work item is named the way this site's project names it
+            // (#251) — a Gutenberg patch must not cite a core.trac ticket that
+            // merely shares its number.
+            const wi = siteWorkItemProvider(meta);
             header = buildProvenanceHeader({
                 handle,
                 event,
                 ticketId: meta.tracTicket,
+                workItemLabel: wi.kind === 'trac' ? 'Ticket' : 'Issue',
+                workItemUrl: meta.tracTicket ? wi.urlFor(meta.tracTicket) : null,
                 // The base the patch was actually diffed against, which on a
                 // ticket branch is the trunk it was born at — not the site's
                 // current trunk, which "Update to latest trunk" may have moved
@@ -766,8 +772,11 @@ ipcMain.handle('github:open-pr', async (event, sitePath, options = {}) => {
     const s = await getStore();
     const meta = (s.get('siteMeta') || {})[sitePath] || {};
     const ticketId = meta.tracTicket;
+    // What the work item is called, and where its pull request goes, both follow
+    // the site's project (#251).
+    const projectType = projectTypeForSite(meta);
     if (!ticketId) {
-        return { ok: false, reason: 'no-ticket', error: 'Link a Trac ticket to this site first.', stage: 'auth' };
+        return { ok: false, reason: 'no-ticket', error: `Link a ${projectType.workItem.label} to this site first — a pull request has to cite one.`, stage: 'auth' };
     }
     const { wporgHandle: handle = null, contributionEvent = null } = s.get('preferences') || {};
 
@@ -778,9 +787,10 @@ ipcMain.handle('github:open-pr', async (event, sitePath, options = {}) => {
         return { ok: false, reason: 'error', error: String(e), stage: 'collect' };
     }
 
+    const provider = siteWorkItemProvider(meta);
     const title = typeof options.title === 'string' && options.title.trim()
         ? options.title.trim()
-        : `Ticket #${ticketId}`;
+        : provider.defaultPrTitle(ticketId);
 
     const result = await openPullRequest({
         token: githubToken,
@@ -789,7 +799,14 @@ ipcMain.handle('github:open-pr', async (event, sitePath, options = {}) => {
         baseSha: collected.baseOid,
         files: collected.files,
         title,
-        body: buildPullRequestBody({ ticketId, handle, event: contributionEvent, notes: options.notes }),
+        project: { upstream: projectType.upstream, branchPrefix: projectType.pr.branchPrefix },
+        body: buildPullRequestBody({
+            ticketId,
+            handle,
+            event: contributionEvent,
+            notes: options.notes,
+            project: { bodyLine: projectType.pr.bodyLine, workItemUrl: provider.urlFor(ticketId) }
+        }),
         onProgress: (stage) => {
             if (!event.sender.isDestroyed()) event.sender.send('github:pr:progress', { sitePath, stage });
         }
@@ -1328,6 +1345,11 @@ const upstreamRepoPath = (meta) => {
     return `${up.owner}/${up.repo}`;
 };
 
+// The work-item provider for a site — Trac tickets for Core, GitHub issues for
+// Gutenberg (#251). Defaults to Trac for a site with no project type.
+const siteWorkItemProvider = (meta) =>
+    workItemProvider(projectTypeForSite(meta).workItem.provider, upstreamRepoPath(meta));
+
 ipcMain.handle('git:list-ticket-patches', async (_e, sitePath) => {
     try {
         const s = await getStore();
@@ -1869,7 +1891,10 @@ ipcMain.handle('sites:set-ticket', async (event, sitePath, ref, options) => with
 		return { ok: true, ticket: null, branch: TRUNK };
 	}
 
-	const parsed = parseTicketRef(raw);
+	// What counts as a work item follows the site's project (#251): a Trac ticket
+	// for Core, a GitHub issue for Gutenberg. Both parse to a number, so the
+	// `ticket/<id>` branch key and every reader of `tracTicket` are unchanged.
+	const parsed = siteWorkItemProvider(await readSiteMeta(sitePath)).parseRef(raw);
 	if (!parsed.ok) return { ok: false, error: parsed.error };
 
 	const blocked = await midSwitchBlock(sitePath);

src/patch-provenance.cjs

@@ -124,15 +124,17 @@ function ticketNumber(ticketId) {
  * recorded.
  *
  * @param {Object}        details
- * @param {string}        [details.handle]      WordPress.org handle, already validated.
- * @param {string}        [details.event]       Where it was written — a WordCamp, a meetup.
+ * @param {string}        [details.handle]        WordPress.org handle, already validated.
+ * @param {string}        [details.event]         Where it was written — a WordCamp, a meetup.
  * @param {number|string} [details.ticketId]
+ * @param {string}        [details.workItemLabel] 'Ticket' (default) or 'Issue'.
+ * @param {string}        [details.workItemUrl]   Defaults to the Trac ticket URL.
  * @param {string}        [details.trunkOid]
- * @param {string}        [details.trunkDate]   ISO timestamp of the base commit.
- * @param {string}        [details.generatedAt] ISO timestamp for "now".
+ * @param {string}        [details.trunkDate]     ISO timestamp of the base commit.
+ * @param {string}        [details.generatedAt]   ISO timestamp for "now".
  * @return {string}
  */
-function buildProvenanceHeader({ handle, event, ticketId, trunkOid, trunkDate, generatedAt } = {}) {
+function buildProvenanceHeader({ handle, event, ticketId, workItemLabel, workItemUrl, trunkOid, trunkDate, generatedAt } = {}) {
 	const lines = [];
 
 	const contributor = field(handle);
@@ -145,8 +147,12 @@ function buildProvenanceHeader({ handle, event, ticketId, trunkOid, trunkDate, g
 	const where = field(event);
 	if (where) lines.push(`# Event: ${where}`);
 
+	// The work item, named the way its own project names it (#251): a Core patch
+	// cites a Trac ticket, a Gutenberg one its GitHub issue. `workItemUrl` is
+	// supplied by the caller, which knows the site's project; without it this
+	// falls back to Trac, which is what every existing caller meant.
 	const ticket = ticketNumber(ticketId);
-	if (ticket) lines.push(`# Ticket: ${ticketUrl(ticket)}`);
+	if (ticket) lines.push(`# ${workItemLabel || 'Ticket'}: ${workItemUrl || ticketUrl(ticket)}`);
 
 	const oid = field(trunkOid);
 	const based = day(trunkDate);

src/project-type.cjs

@@ -54,7 +54,13 @@ const PROJECT_TYPES = {
 		// src/wp-includes layout (patch-plan.cjs mapToSrcLayout).
 		patch: { layout: 'src-layout' },
 
-		workItem: { provider: 'trac' },
+		workItem: {
+			provider: 'trac',
+			// What the panel calls it, and where a newcomer goes to find one.
+			label: 'Trac ticket',
+			browseUrl: 'https://core.trac.wordpress.org/tickets/good-first-bugs',
+			browseLabel: 'Browse good first bugs on Trac'
+		},
 
 		pr: {
 			branchPrefix: 'trac-',
@@ -94,7 +100,12 @@ const PROJECT_TYPES = {
 		// (packages/…); no src-layout rewrite.
 		patch: { layout: 'repo-relative' },
 
-		workItem: { provider: 'github-issue' },
+		workItem: {
+			provider: 'github-issue',
+			label: 'GitHub issue',
+			browseUrl: 'https://github.com/WordPress/gutenberg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Good+First+Issue%22',
+			browseLabel: 'Browse good first issues on GitHub'
+		},
 
 		pr: {
 			branchPrefix: 'fix/issue-',