From a146ad463aaeba4aac070f5112d1e0d4267e2cbb Mon Sep 17 00:00:00 2001 From: "kevin.new" Date: Wed, 25 Feb 2026 00:59:06 +0000 Subject: [PATCH 1/5] feat(mergechecker): adding mergechecker interface with Check --- extension/mergechecker/BUILD.bazel | 9 +++++++++ extension/mergechecker/mergechecker.go | 15 +++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 extension/mergechecker/BUILD.bazel create mode 100644 extension/mergechecker/mergechecker.go diff --git a/extension/mergechecker/BUILD.bazel b/extension/mergechecker/BUILD.bazel new file mode 100644 index 00000000..39e2c33c --- /dev/null +++ b/extension/mergechecker/BUILD.bazel @@ -0,0 +1,9 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "mergechecker", + srcs = ["mergechecker.go"], + importpath = "github.com/uber/submitqueue/extension/mergechecker", + visibility = ["//visibility:public"], + deps = ["//entity"], +) diff --git a/extension/mergechecker/mergechecker.go b/extension/mergechecker/mergechecker.go new file mode 100644 index 00000000..98c773cc --- /dev/null +++ b/extension/mergechecker/mergechecker.go @@ -0,0 +1,15 @@ +package mergechecker + +import ( + "context" + + "github.com/uber/submitqueue/entity" +) + +// MergeChecker predicts whether a request's changes can merge cleanly. +type MergeChecker interface { + // Check is a fail-fast validation that optimistically assesses the + // mergeability of the request. A positive result does not guarantee + // that the changes will apply cleanly at merge finalization time. + Check(ctx context.Context, request entity.Request) (bool, error) +} From 5769b146f572af46ac107fb4e7298e4f330e6338 Mon Sep 17 00:00:00 2001 From: "kevin.new" Date: Wed, 25 Feb 2026 01:47:24 +0000 Subject: [PATCH 2/5] removed the bool return; added sentinel error to return instead --- extension/mergechecker/mergechecker.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/extension/mergechecker/mergechecker.go b/extension/mergechecker/mergechecker.go index 98c773cc..83b76808 100644 --- a/extension/mergechecker/mergechecker.go +++ b/extension/mergechecker/mergechecker.go @@ -2,14 +2,19 @@ package mergechecker import ( "context" + "errors" "github.com/uber/submitqueue/entity" ) +// ErrUnmergeable is returned by Check when the request's changes are not mergeable. +var ErrUnmergeable = errors.New("request is not mergeable") + // MergeChecker predicts whether a request's changes can merge cleanly. type MergeChecker interface { // Check is a fail-fast validation that optimistically assesses the - // mergeability of the request. A positive result does not guarantee + // mergeability of the request. A nil result does not guarantee // that the changes will apply cleanly at merge finalization time. - Check(ctx context.Context, request entity.Request) (bool, error) + // Returns ErrUnmergeable if the changes are not mergeable. + Check(ctx context.Context, request entity.Request) error } From 14ec23b5d48c282e1404bf8bb6484f458de9cb79 Mon Sep 17 00:00:00 2001 From: "kevin.new" Date: Wed, 25 Feb 2026 02:09:45 +0000 Subject: [PATCH 3/5] bring back bool --- extension/mergechecker/mergechecker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/mergechecker/mergechecker.go b/extension/mergechecker/mergechecker.go index 83b76808..e0590b51 100644 --- a/extension/mergechecker/mergechecker.go +++ b/extension/mergechecker/mergechecker.go @@ -13,8 +13,8 @@ var ErrUnmergeable = errors.New("request is not mergeable") // MergeChecker predicts whether a request's changes can merge cleanly. type MergeChecker interface { // Check is a fail-fast validation that optimistically assesses the - // mergeability of the request. A nil result does not guarantee + // mergeability of the request. A positive result does not guarantee // that the changes will apply cleanly at merge finalization time. // Returns ErrUnmergeable if the changes are not mergeable. - Check(ctx context.Context, request entity.Request) error + Check(ctx context.Context, request entity.Request) (bool, error) } From 8a1517eedf9af361a4283d0f3b6b34bb01820c42 Mon Sep 17 00:00:00 2001 From: "kevin.new" Date: Wed, 25 Feb 2026 02:37:44 +0000 Subject: [PATCH 4/5] remove the sentinel error --- extension/mergechecker/mergechecker.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/extension/mergechecker/mergechecker.go b/extension/mergechecker/mergechecker.go index e0590b51..98c773cc 100644 --- a/extension/mergechecker/mergechecker.go +++ b/extension/mergechecker/mergechecker.go @@ -2,19 +2,14 @@ package mergechecker import ( "context" - "errors" "github.com/uber/submitqueue/entity" ) -// ErrUnmergeable is returned by Check when the request's changes are not mergeable. -var ErrUnmergeable = errors.New("request is not mergeable") - // MergeChecker predicts whether a request's changes can merge cleanly. type MergeChecker interface { // Check is a fail-fast validation that optimistically assesses the // mergeability of the request. A positive result does not guarantee // that the changes will apply cleanly at merge finalization time. - // Returns ErrUnmergeable if the changes are not mergeable. Check(ctx context.Context, request entity.Request) (bool, error) } From 39065da3557ede8889a5e97b2ec32e9e90fbdee5 Mon Sep 17 00:00:00 2001 From: "kevin.new" Date: Wed, 25 Feb 2026 02:59:49 +0000 Subject: [PATCH 5/5] update CLAUDE.md with instructions to not use errs for version control; Updated merge checker to use a result struct with bool nested --- CLAUDE.md | 1 + extension/mergechecker/mergechecker.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 68626bf2..86bc20ad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -202,3 +202,4 @@ See [doc/howto/TESTING.md](doc/howto/TESTING.md) for full testing guide. 1. **Structured logging** — `zap.SugaredLogger` with `Debugw`/`Infow`/`Errorw(msg, key, val, ...)`. Never unstructured methods. 2. **Interfaces for behavior, structs for data** — use interfaces for behavioral contracts (Consumer, Controller, Storage). Use structs for data containers, configs, and registries (TopicRegistry, SubscriptionConfig). 3. **Value types over pointers** — prefer value types for structs, configs, and return values. Use `(T, bool)` to signal absence instead of `*T`. Pointers only when mutation or shared ownership is needed. +4. **Errors for failures, not control flow** — reserve `error` returns for unexpected or infrastructure failures. Use result types (structs, bools) for expected outcomes like `(Result, error)` or `(T, bool)`. Avoid sentinel errors that represent non-failure states. diff --git a/extension/mergechecker/mergechecker.go b/extension/mergechecker/mergechecker.go index 98c773cc..ba1690ac 100644 --- a/extension/mergechecker/mergechecker.go +++ b/extension/mergechecker/mergechecker.go @@ -11,5 +11,11 @@ type MergeChecker interface { // Check is a fail-fast validation that optimistically assesses the // mergeability of the request. A positive result does not guarantee // that the changes will apply cleanly at merge finalization time. - Check(ctx context.Context, request entity.Request) (bool, error) + Check(ctx context.Context, request entity.Request) (Result, error) +} + +// Result holds the outcome of a merge check. +type Result struct { + // Mergeable is true if the request's changes are expected to merge cleanly. + Mergeable bool }