Skip to content

refactor: consolidate shared code under platform/#257

Merged
behinddwalls merged 3 commits into
mainfrom
refactor
Jun 17, 2026
Merged

refactor: consolidate shared code under platform/#257
behinddwalls merged 3 commits into
mainfrom
refactor

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

Move the cross-domain shared trees out of the repo root into a single platform/ umbrella. Domain-scoped trees (submitqueue/, stovepipe/, runway/) keep their own core/, entity/, extension/.

From To
core/{errs,metrics,consumer} platform/{errs,metrics,consumer}
core/httpclient platform/http
entity/* (change, mergestrategy, messagequeue) platform/base/*
extension/* (counter, messagequeue) platform/extension/*

Details:

  • platform/http: package renamed httpclient -> http; net/http aliased as nethttp inside the package; callers that also import net/http use a phttp alias.
  • platform/base: root doc package renamed entity -> base; change / mergestrategy / messagequeue preserved as subpackages.
  • Rewrites all Go import paths and Bazel labels; updates Makefile schema/admin paths and go:generate roots.
  • Docs refreshed to the platform/ layout (CLAUDE.md, package READMEs, RFCs), documenting current state only.

Stacked on #256 (hermetic Go SDK). Review/merge that first.

Test plan

  • make gazelle clean (BUILD files in sync)
  • make build — 201 targets build (hermetic)
  • make test — 54/54 unit tests pass

Made with Cursor

Test Plan

Issues

Stack

  1. @ refactor: consolidate shared code under platform/ #257
  2. refactor(proto): move protos to top-level api/ tree by domain/service #258
  3. docs(rfc): add message queue contract RFC #259
  4. feat(api/runway): add external message queue contract as JSON Schema #260

behinddwalls and others added 2 commits June 16, 2026 23:25
Drop `go_sdk.host()` so toolchain resolution no longer instantiates the
host Go SDK (which fails when `go` is not on PATH). The SDK version pinned
in go.mod is downloaded by rules_go, plus a linux/amd64 SDK for the Docker
cross-compile targets. Also remove the redundant version-pinned host
download already covered by `from_file`.

Co-authored-by: Cursor <cursoragent@cursor.com>
Move the cross-domain shared trees out of the repo root into a single
platform/ umbrella:

- core/{errs,metrics,consumer} -> platform/{errs,metrics,consumer}
- core/httpclient              -> platform/http (package renamed to http,
                                  net/http aliased as nethttp; callers that
                                  also import net/http use the phttp alias)
- entity/*                     -> platform/base/* (root doc package renamed
                                  entity -> base; change, mergestrategy,
                                  messagequeue preserved as subpackages)
- extension/*                  -> platform/extension/*

Rewrites all import paths and Bazel labels, updates Makefile schema/admin
paths, and refreshes docs (CLAUDE.md, READMEs, RFCs) to the platform/
layout. Domain-scoped trees (submitqueue/, stovepipe/, runway/) keep their
own core/entity/extension.

Co-authored-by: Cursor <cursoragent@cursor.com>
@behinddwalls behinddwalls requested review from a team and sbalabanov as code owners June 17, 2026 06:26
Re-run goimports across packages touched by the platform/ consolidation
so the gofmt/goimports lint check passes.

Co-authored-by: Cursor <cursoragent@cursor.com>
@behinddwalls behinddwalls merged commit f393a13 into main Jun 17, 2026
27 checks passed
@behinddwalls behinddwalls deleted the refactor branch June 17, 2026 17:27
behinddwalls added a commit that referenced this pull request Jun 17, 2026
…#258)

## Summary
Stacked on top of `refactor` (#257). Moves all proto wire contracts into
a single top-level `api/` tree organized by `domain/service`, and lays
the groundwork for upcoming messagequeue payload contracts.

- Relocate each service's `proto/` and `protopb/` dirs to
`api/{domain}/{service}/` (history-preserving renames). New importpath:
`github.com/uber/submitqueue/api/{domain}/{service}/protopb`.
- Update `go_package`, all Bazel importpaths, root `gazelle:resolve`
hints, consumer `deps`, and all Go imports.
- Generalize the `tool/proto` codegen rule (`srcs` list) and the `make
proto` copy loop to support **multiple `.proto` files per service
package**, so messagequeue contracts can later live alongside each
service's RPC contract under the same `api/{domain}/{service}/proto/`
dir.
- Update docs (`CLAUDE.md` project layout / import paths / proto-gen
workflow, `example/README.md`).

The proto `package` declarations are intentionally left unchanged (only
`go_package` moved) to avoid churning wire-level type names.

## Test plan

- [x] `make build`
- [x] `make test` (54/54 pass)
- [x] `make check-gazelle`
- [x] `make lint`
- [x] `make check-tidy`
- [x] `make proto` produces zero drift vs committed stubs

Made with [Cursor](https://cursor.com)

## Stack
1. @ #258
1. #261
1. #262

Co-authored-by: Cursor <cursoragent@cursor.com>
behinddwalls added a commit that referenced this pull request Jun 17, 2026
## Summary

### Why?

The "Rebase Stacked PRs" job silently no-ops when the repo's "Automatically delete head branches" setting is on. On merge, GitHub deletes the merged head branch, which auto-retargets every child PR's base from the merged head branch to the merged base (e.g. main) *before* the job runs. The job discovers children via `gh pr list --base <merged-head>`, which then returns nothing — so it rebases nothing, force-pushes nothing, and still reports success. Run #257 (and #256 before it) did exactly this, leaving the downstream stack (#258/#259/#260) with doubled diffs that had to be rebased by hand.

The fix is to keep "Automatically delete head branches" off (now done at the repo level) and let this workflow own head-branch cleanup. With the setting off, GitHub leaves child bases untouched, the lookup finds them, and the job rebases the stack and then deletes the merged branch itself — for both stacked and non-stacked PRs.

### What?

- Document the hard requirement that "Automatically delete head branches" stays off, with the retarget-race rationale, in the workflow header.
- Log a clear line when a merge has no child PRs instead of returning silently.
- Clarify the branch-deletion step: it runs on every merged PR (stacked or not) and is skipped only when a child rebase failed; fix the misleading "All stacked PRs rebased successfully" message that printed even on no-op runs.

No behavior change on the happy path — the job already deleted the merged branch on success; this makes the intent explicit and the logs legible.

## Test Plan

- ✅ `python3 -c "import yaml; yaml.safe_load(open('.github/workflows/rebase-stack.yml'))"` — YAML parses.
- Confirmed `delete_branch_on_merge` is `false` via `gh api repos/uber/submitqueue`.
- Post-merge: the next Rebase Stack run should log child rebases or "No open child PRs … nothing to rebase", then "Deleting merged branch …" and actually remove it.

Co-authored-by: Cursor <cursoragent@cursor.com>
behinddwalls added a commit that referenced this pull request Jun 17, 2026
## Summary

### Why?

The "Rebase Stacked PRs" job was silently no-opping: with the repo's
"Automatically delete head branches" setting on, GitHub deleted the
merged head branch on merge, which auto-retargeted every child PR's base
from the merged head branch to the merged base (e.g. `main`) *before*
the job ran. Its child lookup (`gh pr list --base <merged-head>`) then
found nothing, so it rebased nothing and still went green. Run #257 (and
#256 before it) did exactly this, leaving #258/#259/#260 with doubled
diffs that had to be rebased by hand.

The repo setting is now off, making this workflow the sole owner of
head-branch cleanup. But that exposed a second gap: the job only deleted
the merged branch at the end of its own run. When a child rebase
conflicts, the run intentionally keeps the merged branch (the child
still bases on it) and never fires again — so after the author manually
rebases and retargets the child, nothing ever deletes the orphaned
merged branch.

### What?

- Document the hard requirement that "Automatically delete head
branches" stays **off**, with the retarget-race rationale, in the
workflow header.
- Replace the outcome-gated single-branch delete with an invariant-based
sweep, `cleanup_orphaned_merged_branches`, that runs on every
invocation: for each recently merged PR whose head branch still exists,
delete it **iff** no open PR references it as a base or head. This:
- deletes the just-merged branch on a clean run (children retargeted
away),
- **keeps** it when an immediate child rebase failed (that child still
bases on it — deleting it would retarget the child to `main` and
recreate the broken diff),
- reaps branches stranded by an earlier conflicted run on the next
merge, once their children were manually fixed.
- Branch existence is snapshotted in one `git ls-remote`; the merged
base (e.g. `main`) is never touched. Conflicted runs also emit a
`::warning::` for visibility while still exiting non-fatally, and
no-stack merges log a clear line.

## Test Plan

- ✅ `yaml.safe_load` parses the workflow.
- ✅ `bash -n` on the extracted `run:` script.
- ✅ Simulated the sweep loop with mock data (dedup, skip-gone,
skip-base, consider-delete all behave correctly).
- Post-merge: the next Rebase Stack run should log child rebases (or "No
open child PRs … nothing to rebase"), then a sweep that deletes the
merged branch when nothing depends on it.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants