Skip to content

Commit 3e75dd9

Browse files
authored
refactor(entity): promote Change + ChangeID to shared entity/change (#240)
## Summary ### Why? `Change` (a code change identified by provider URIs) and the GitHub/Phabricator `ChangeID` parsers are pure identity-and-parsing types with no domain semantics and no persisted-as-shared schema. They are the one cluster genuinely common to every code-change pipeline — SubmitQueue today, and Stovepipe / Runway / Tango alignment going forward — so they belong in the shared top-level `entity/` tree rather than inside `submitqueue/entity/`. This is deliberately the *only* promotion: facts a domain persists (the change-provider model embedded in `ChangeRecord`), state machines, and pipeline types stay domain-local, since sharing them would couple every domain's storage schema and bend their state machines toward SubmitQueue's. ### What? Moves the canonical `Change` struct out of `submitqueue/entity/request.go` into a new `entity/change` package, and relocates the parser packages to `entity/change/github` and `entity/change/phabricator` (preserving git history). No alias shim: every consumer now references `change.Change` and imports the new path directly. JSON tags are unchanged, so queue payloads (`Request`, `LandRequest`, `PushResult`) remain byte-identical across a deploy. `submitqueue/entity` keeps its full pipeline and facts; `Request`, `LandRequest`, and `ChangeOutcome` now embed `change.Change`. ## Test Plan - ✅ `make build` - ✅ `make test` (51/51 pass) - ✅ `make fmt` / `make gazelle` (converged, no further changes) - ✅ `make check-tidy` (go.mod / MODULE.bazel unchanged — internal move, no new deps)
1 parent 5cbd41a commit 3e75dd9

74 files changed

Lines changed: 239 additions & 146 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

entity/change/BUILD.bazel

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
load("@rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "change",
5+
srcs = ["change.go"],
6+
importpath = "github.com/uber/submitqueue/entity/change",
7+
visibility = ["//visibility:public"],
8+
)

entity/change/change.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Package change holds the shared code-change identity used across SubmitQueue,
16+
// Stovepipe, and other repo-local domains. A Change names code to act on via
17+
// provider URIs; the URI parsers live in the github and phabricator subpackages.
18+
package change
19+
20+
// Change represents a code change identified by URIs from a code change provider (e.g., GitHub Pull Request, Phabricator Diff).
21+
// The provider is extracted from the URI scheme. The object is immutable after creation.
22+
type Change struct {
23+
// URIs identifies the change(s) to land (RFC 3986 compliant).
24+
// The scheme identifies the change provider, and the path contains provider-specific resource identifiers.
25+
//
26+
// GitHub is supported by default (though other providers can be added):
27+
// Template: "<scheme>://<org>/<repo>/pull/<pr>/<head_commit_sha>"
28+
// Example: "github://uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"
29+
// Schemes: "github", "ghe", "ghes". Head commit SHA must be full 40-char lowercase hex.
30+
//
31+
URIs []string `json:"uris"`
32+
}
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 = "github",
55
srcs = ["change_id.go"],
6-
importpath = "github.com/uber/submitqueue/submitqueue/entity/github",
6+
importpath = "github.com/uber/submitqueue/entity/change/github",
77
visibility = ["//visibility:public"],
88
)
99

File renamed without changes.

submitqueue/entity/phabricator/BUILD.bazel renamed to entity/change/phabricator/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 = "phabricator",
55
srcs = ["change_id.go"],
6-
importpath = "github.com/uber/submitqueue/submitqueue/entity/phabricator",
6+
importpath = "github.com/uber/submitqueue/entity/change/phabricator",
77
visibility = ["//visibility:public"],
88
)
99

submitqueue/entity/phabricator/change_id.go renamed to entity/change/phabricator/change_id.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ var diffPattern = regexp.MustCompile(`^[1-9]\d*$`)
5454
// code state, analogous to GitHub's head commit SHA on a pull request.
5555
type ChangeID struct {
5656
// Scheme captures the URI scheme. Always "phab" today; kept as a field so
57-
// the parsed form mirrors entity/github.ChangeID and so future variants
57+
// the parsed form mirrors entity/change/github.ChangeID and so future variants
5858
// (e.g., a separate scheme per Phabricator install) are a non-breaking add.
5959
Scheme string
6060
// RevisionID is the numeric portion of the Phabricator revision identifier
File renamed without changes.

submitqueue/core/changeset/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ go_library(
99
importpath = "github.com/uber/submitqueue/submitqueue/core/changeset",
1010
visibility = ["//visibility:public"],
1111
deps = [
12+
"//entity/change",
1213
"//submitqueue/entity",
1314
"//submitqueue/extension/storage",
1415
],
@@ -19,6 +20,7 @@ go_test(
1920
srcs = ["resolver_test.go"],
2021
embed = [":changeset"],
2122
deps = [
23+
"//entity/change",
2224
"//submitqueue/entity",
2325
"//submitqueue/extension/storage/mock",
2426
"@com_github_stretchr_testify//assert",

submitqueue/core/changeset/changeset.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ package changeset
2626
import (
2727
"context"
2828

29+
"github.com/uber/submitqueue/entity/change"
2930
"github.com/uber/submitqueue/submitqueue/entity"
3031
)
3132

@@ -39,7 +40,7 @@ type Resolver interface {
3940
// changes (URIs only; no change-store read), in batch.Contains order. A batch
4041
// with no requests yields an empty slice. Used by the build (base/head) and
4142
// merge stages.
42-
ChangesForBatch(ctx context.Context, batch entity.Batch) ([]entity.Change, error)
43+
ChangesForBatch(ctx context.Context, batch entity.Batch) ([]change.Change, error)
4344

4445
// DetailedForBatch resolves a batch into its normalized, batch-level view:
4546
// one entity.ChangeInfo per claimed URI (URI plus the provider details read

0 commit comments

Comments
 (0)