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

A pull's green is the merge's green: checks run on the merge with its base, a conflict is said first, and a stale green never reads as ready #1133

open not picked up yet opened this
opened this

Today the owner's inbox showed trevato/cs#207 as a green pull on him, and the owner approved it on that green. It had conflicted with main for weeks (22 commits moved under it) and its reviewer had asked for changes. The check was honest about what it tested — the head commit, alone — and wrong about what the person reads it as: that the change is ready to land.

Done looks like:

  • [ ] A pull's check runs on the merge of its head into the current base (a temporary merge commit, the way a merge queue tests), not on the head alone. When the base moves, the check is stale: the pull says "checked against an older main" and re-checks on its own when a slot is free, newest base first.
  • [ ] A pull that does not merge says so before anything else — on the pull page, in the inbox row, in the feed, on the checks line ("Conflicts in 2 files" where "Checks passed" would be) and in the API (mergeable, conflicts on the pull JSON). A conflicted pull is never shown with a green mark anywhere.
  • [ ] A review's approval is recorded against the head and the base it was given for; when either moves, the approval reads "approved at 28818e8 against 598c302" and the inbox row says "moved since approval".
  • [ ] GET /api/v1/repos/:owner/:name/pulls/:number carries mergeable, conflicts, checkedAgainst and approvedAt.
  • [ ] Tests: a pull whose base moves under it loses its green until re-checked; a conflicted pull's inbox row and checks line say conflicts; the API fields are present.

Files I expect: src/checks/service.ts (merge-ref check), src/core/pulls.ts (mergeStatus), src/web/pages/pull.ts (checksLine), src/web/pages/site.ts (inbox rows), src/http/api.ts.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs