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

A forge preview takes owner looks of an app's page, so a pull touching it shows main beside head #9784

Merged opened this

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

Merged

A preview of the forge now seeds one app, and its page is taken at desktop and phone as the owner. The pull's pair at 1440 and 390 only shows once main has its own looks of this page. I didn't use `keep_plan`, so the issue has no plan on it.

shipped production is serving it deploy ef71500

opened this

A preview of the forge now seeds one app, and its page is taken at desktop and phone as the owner. The pull's pair at 1440 and 390 only shows once main has its own looks of this page. I didn't use keep_plan, so the issue has no plan on it.

Tests pass for the new test and the existing ones that use the seed or the owner-looks list: the preview-looks, looks-owner, flows, fit and signup-gate tests. bun scripts/gate.ts and bun x tsc --noEmit were clean. I didn't run the whole suite, since the forge runs it on the push. Pushed.

  • Page choice: the issue says /apps/:name, but no such route exists. An app's page is its repository's home (/:owner/:name), and /:owner/:name/app only redirects there. Making the existing sample repo an app would have turned its home into the app page and changed that look. So the seed makes a second repository, preview-admin/hello, with an app record and a README commit, and PREVIEW_OWNER_LOOKS names /preview-admin/hello.
  • Lights: that adds the desktop and phone looks, and the list already gives every page light and dark pictures.
  • 200 check: the seed renders that page in-process as the owner, with no server. It returns an error if the status isn't 200, so the preview doesn't boot on a bad seed. I didn't run a failing case to see it refuse.
  • No process or network: only the record is seeded. Nothing starts and no network is reached, and the page says the app isn't running.
  • Test: test/preview-app-look.test.ts checks the list holds the page at both widths and that the seed made exactly one app record. It would fail without this change, since the constants don't exist.
  • Previews already seeded: a preview seeded before this change won't get the app, because the seed runs once per data directory. A fresh preview or a new data directory does.
  • Not done: no pixel check of the app page. The 200 check covers it answering, not how it looks.

The change is in src/spawn/preview-looks.ts and src/spawn/preview-seed.ts.

Closes #9782

commented

A preview of the forge now seeds one app, and its page is taken at desktop and phone as the owner. The pull's pair at 1440 and 390 only shows once main has its own looks of this page. I didn't use keep_plan, so the issue has no plan on it.

Tests pass for the new test and the existing ones that use the seed or the owner-looks list: the preview-looks, looks-owner, flows, fit and signup-gate tests. bun scripts/gate.ts and bun x tsc --noEmit were clean. I didn't run the whole suite, since the forge runs it on the push. Pushed.

  • Page choice: the issue says /apps/:name, but no such route exists. An app's page is its repository's home (/:owner/:name), and /:owner/:name/app only redirects there. Making the existing sample repo an app would have turned its home into the app page and changed that look. So the seed makes a second repository, preview-admin/hello, with an app record and a README commit, and PREVIEW_OWNER_LOOKS names /preview-admin/hello.
  • Lights: that adds the desktop and phone looks, and the list already gives every page light and dark pictures.
  • 200 check: the seed renders that page in-process as the owner, with no server. It returns an error if the status isn't 200, so the preview doesn't boot on a bad seed. I didn't run a failing case to see it refuse.
  • No process or network: only the record is seeded. Nothing starts and no network is reached, and the page says the app isn't running.
  • Test: test/preview-app-look.test.ts checks the list holds the page at both widths and that the seed made exactly one app record. It would fail without this change, since the constants don't exist.
  • Previews already seeded: a preview seeded before this change won't get the app, because the seed runs once per data directory. A fresh preview or a new data directory does.
  • Not done: no pixel check of the app page. The 200 check covers it answering, not how it looks.

The change is in src/spawn/preview-looks.ts and src/spawn/preview-seed.ts.

Approved

Approved. The preview serves /preview-admin/hello with 200, the seed makes exactly one app, and the list names the page. Two things I would do next: (1) test the refusal itself, since no test shows the seed failing when the page would not answer 200; (2) replace the hand-built as Ctx object in seedApp with a real ctx builder, and tidy the over-long header line in preview-looks.ts. Note: no hello pictures were among the looks I was given for this head, so confirm the looks service picks the page up on the next push.

commented

Left #9782 open: 1 unticked item remain (Once main's looks of it exist, a pull that touches src/web/pages/apps-name.ts shows the pair at 1440 and 390.).

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs