Skip to content

Commit de22c87

Browse files
committed
refactor(platform)!: move CI clients out of extension/ to platform/
## Summary ### Why? `platform/extension/buildrunner/{buildkite,githubactions}` were filed as extensions, but neither is one. Per [CLAUDE.md](CLAUDE.md), an `extension/{ext}` package holds the behavioral interface, its `Config`, and the `Factory` interface — `platform/extension/buildrunner/` had no Go package at its root at all, just two impl directories under an empty namespace. Both packages say so in their own doc comments: "It intentionally holds no BuildRunner interface or domain entity types." What they actually are is vendor HTTP clients over `platform/http` — REST calls plus provider-specific vocabulary (Buildkite's state strings, GitHub Actions' status/conclusion pair, id encoding). That makes them siblings of `platform/http` and `platform/errs`, not extensions. The real `BuildRunner` contracts already live where they belong, at `submitqueue/extension/buildrunner` and `stovepipe/extension/buildrunner`; these clients are what those extensions' backends wrap. ### What? Moves both packages up to `platform/buildkite` and `platform/githubactions` and deletes the now-empty `platform/extension/buildrunner/`. Pure relocation — no behavior, no signatures, no test logic changed. Import paths and Bazel labels updated across the four consumer packages (`{submitqueue,stovepipe}/extension/buildrunner/{buildkite,githubactions}`), which keep their `platformbuildkite`/`platformgithubactions` aliases. `platform/README.md` gains an entry for the two clients and a line on the `platform/` vs `platform/extension/` test, so the next vendor client lands in the right place. In [doc/rfc/stovepipe/steps/build.md](doc/rfc/stovepipe/steps/build.md), only the *adopted* option is repathed. The rejected alternatives there describe a shared `BuildRunner` **interface**, which genuinely would have been an extension — so their `platform/extension/buildrunner` references are correct as written and left intact. The adopted option now also records why the shipped package is not an extension: it is precisely the option that declines to define a shared interface. ## Test Plan - ✅ `bazel test //platform/buildkite/... //platform/githubactions/... //submitqueue/extension/buildrunner/... //stovepipe/extension/buildrunner/...` — 8/8 pass - ✅ `bazel build //...` — 309 targets - ✅ `make fmt` (no changes), `make lint`, `make check-gazelle`, `make check-tidy`
1 parent 61db11f commit de22c87

25 files changed

Lines changed: 29 additions & 28 deletions

File tree

doc/rfc/stovepipe/steps/build.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,10 @@ Several shapes for sharing the `BuildRunner` contract across domains were raised
230230
- **Shared `Status`/`Cancel` via a `platform/extension/buildrunner.StatusCanceller` sub-interface, with `BuildID`/`BuildStatus`/`BuildMetadata` promoted to `platform/base`.** An earlier draft of this doc adopted exactly this: since both domains poll and cancel by the same opaque, runner-minted id with the same async semantics, `Status`/`Cancel` moved to a shared interface embedded in each domain's `BuildRunner`, with the supporting types promoted to `platform/base` so both sides used the same Go types (a dual-implementing backend would then satisfy both interfaces through one embedded method set).
231231

232232
Trade-offs: set aside on review — splitting one conceptual contract (`Trigger` + `Status` + `Cancel`) across two packages (`platform/extension/buildrunner` for two of the three methods, `{domain}/extension/buildrunner` for the third) fragments a single interface across an ownership boundary for a resemblance that isn't yet load-bearing: SubmitQueue is the only existing consumer of the "shared" half today, and the promotion cost — migrating SubmitQueue's already-shipped controllers, storage, and protobuf mappings onto the shared type — bought less than keeping each domain's `BuildRunner` whole and pushing reuse down to the implementation layer instead, per the option below.
233-
- **Shared backend under `platform`, thin per-domain contracts (adopted).** House the Buildkite / CI-gateway implementation once under `platform/extension` and let each domain define its own contract over it:
233+
- **Shared backend under `platform`, thin per-domain contracts (adopted).** House the Buildkite / CI-gateway implementation once under `platform/` and let each domain define its own contract over it. It sits at `platform/{backend}`, beside `platform/http`, rather than under `platform/extension/` — precisely because this option declines to define a shared interface, the package is a vendor client with no interface, `Config`, or `Factory`, so it is platform plumbing rather than an extension. Only the rejected alternatives above would have earned a `platform/extension/buildrunner` package.
234234

235235
```go
236-
// platform/extension/buildrunner/buildkite — shared HTTP client, auth, poll loop
236+
// platform/buildkite — shared HTTP client, auth, poll loop
237237
package buildkite
238238

239239
type Client struct{ /* ... */ }

platform/README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ Cross-domain packages shared by SubmitQueue, Stovepipe, and other services in th
88
- **metrics/** — Tally helpers with error-aware tagging via `platform/errs`.
99
- **consumer/** — Queue consumer framework (`consumer.Controller`, registry, DLQ wiring).
1010
- **http/** — Small HTTP client helpers (e.g. base-URL `RoundTripper`). Go import path: `github.com/uber/submitqueue/platform/http`; package name is `http`. Callers that also import `net/http` should import this package with an alias (for example `phttp "github.com/uber/submitqueue/platform/http"`) and use `phttp.NewClient`.
11+
- **buildkite/**, **githubactions/** — Vendor CI clients over `platform/http`: the REST calls and provider-specific vocabulary (state strings, id encoding) shared by every domain's `BuildRunner` backend. They deliberately define no interface, so they are plumbing rather than extensions — each domain keeps its own `BuildRunner` contract under `{domain}/extension/buildrunner` and adapts these clients to it.
1112
- **base/** — Shared domain entities (`change`, `messagequeue`, and related subpackages). Root package `base` is documentation-only.
12-
- **extension/** — Shared extension interfaces and implementations reused across domains (`counter`, `messagequeue`, and backends such as `mysql`).
13+
- **extension/** — Shared extension interfaces and implementations reused across domains (`counter`, `messagequeue`, and backends such as `mysql`). A package belongs here only if it defines a behavioral interface with its `Config` and `Factory` interface; a vendor client with no interface belongs directly under `platform/`.
1314

1415
Domain-scoped infrastructure and extensions stay under each domain (for example `submitqueue/core/`, `submitqueue/extension/`).

platform/extension/buildrunner/buildkite/BUILD.bazel renamed to platform/buildkite/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")
33
go_library(
44
name = "go_default_library",
55
srcs = ["client.go"],
6-
importpath = "github.com/uber/submitqueue/platform/extension/buildrunner/buildkite",
6+
importpath = "github.com/uber/submitqueue/platform/buildkite",
77
visibility = ["//visibility:public"],
88
deps = ["//platform/http:go_default_library"],
99
)

platform/extension/buildrunner/buildkite/README.md renamed to platform/buildkite/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Buildkite client
22

3-
Shared HTTP client and Buildkite-specific facts for every domain's Buildkite-backed `BuildRunner`. There is no `BuildRunner` interface here by design — each domain (`submitqueue`, `stovepipe`, ...) defines its own `BuildRunner` and its own `BuildStatus`, and adapts this package's `State` to it. See [`doc/rfc/stovepipe/steps/build.md`](../../../../doc/rfc/stovepipe/steps/build.md#alternatives-considered-for-sharing-the-contract) for why the contract stays per-domain while the backend is shared.
3+
Shared HTTP client and Buildkite-specific facts for every domain's Buildkite-backed `BuildRunner`. There is no `BuildRunner` interface here by design — each domain (`submitqueue`, `stovepipe`, ...) defines its own `BuildRunner` and its own `BuildStatus`, and adapts this package's `State` to it. See [`doc/rfc/stovepipe/steps/build.md`](../../doc/rfc/stovepipe/steps/build.md#alternatives-considered-for-sharing-the-contract) for why the contract stays per-domain while the backend is shared.
44

55
## What lives here
66

@@ -17,7 +17,7 @@ Both wrap a `*Client` built at the wiring layer; this package never constructs o
1717

1818
## How the same `Client` stays safe to share across two different checkout strategies
1919

20-
SubmitQueue's build materializes state that doesn't exist yet — the runner resolves a batch DAG into composite base/head commits by applying patches. Stovepipe's build checks out a commit that already exists on trunk and, for an incremental build, diffs it against a baseline. These are different problems at the CI-pipeline level (see [build.md's "Why separate contracts"](../../../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts)), yet both go through the same `Client.CreateBuild` call — the `Client` never inspects `CreateBuildRequest.Env` or picks a strategy, so there is nothing here that needs to "know" which pattern applies.
20+
SubmitQueue's build materializes state that doesn't exist yet — the runner resolves a batch DAG into composite base/head commits by applying patches. Stovepipe's build checks out a commit that already exists on trunk and, for an incremental build, diffs it against a baseline. These are different problems at the CI-pipeline level (see [build.md's "Why separate contracts"](../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts)), yet both go through the same `Client.CreateBuild` call — the `Client` never inspects `CreateBuildRequest.Env` or picks a strategy, so there is nothing here that needs to "know" which pattern applies.
2121

2222
The split happens entirely outside this package, at two layers below it:
2323

platform/extension/buildrunner/buildkite/client.go renamed to platform/buildkite/client.go

File renamed without changes.

platform/extension/buildrunner/buildkite/client_test.go renamed to platform/buildkite/client_test.go

File renamed without changes.

platform/extension/buildrunner/githubactions/BUILD.bazel renamed to platform/githubactions/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")
33
go_library(
44
name = "go_default_library",
55
srcs = ["client.go"],
6-
importpath = "github.com/uber/submitqueue/platform/extension/buildrunner/githubactions",
6+
importpath = "github.com/uber/submitqueue/platform/githubactions",
77
visibility = ["//visibility:public"],
88
deps = ["//platform/http:go_default_library"],
99
)

platform/extension/buildrunner/githubactions/README.md renamed to platform/githubactions/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# GitHub Actions client
22

3-
Shared HTTP client and GitHub Actions-specific facts for every domain's GitHub Actions-backed `BuildRunner`. There is no `BuildRunner` interface here by design — each domain (`submitqueue`, `stovepipe`, ...) defines its own `BuildRunner` and its own `BuildStatus`, and adapts this package's `RunStatus` to it. Mirrors [`platform/extension/buildrunner/buildkite`](../buildkite/README.md)'s split; see that package's README and [`doc/rfc/stovepipe/steps/build.md`](../../../../doc/rfc/stovepipe/steps/build.md#alternatives-considered-for-sharing-the-contract) for the shared rationale.
3+
Shared HTTP client and GitHub Actions-specific facts for every domain's GitHub Actions-backed `BuildRunner`. There is no `BuildRunner` interface here by design — each domain (`submitqueue`, `stovepipe`, ...) defines its own `BuildRunner` and its own `BuildStatus`, and adapts this package's `RunStatus` to it. Mirrors [`platform/buildkite`](../buildkite/README.md)'s split; see that package's README and [`doc/rfc/stovepipe/steps/build.md`](../../doc/rfc/stovepipe/steps/build.md#alternatives-considered-for-sharing-the-contract) for the shared rationale.
44

55
## What lives here
66

platform/extension/buildrunner/githubactions/client.go renamed to platform/githubactions/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
// BuildRunner interface or domain entity types — each domain (submitqueue,
1919
// stovepipe, ...) defines its own BuildRunner and its own BuildStatus, and
2020
// adapts this package's RunStatus to it. See
21-
// platform/extension/buildrunner/buildkite's README for the analogous
21+
// platform/buildkite's README for the analogous
2222
// rationale applied to the Buildkite backend.
2323
package githubactions
2424

platform/extension/buildrunner/githubactions/client_test.go renamed to platform/githubactions/client_test.go

File renamed without changes.

0 commit comments

Comments
 (0)