Context
In #197, the extension fakes inject failures via an ad-hoc sq-fake=<token>
marker embedded in a change URI, parsed by the test-only core/fakemarker
package (Token([]string) / TokenInChanges([]entity.Change)).
Reviewing that PR, @sbalabanov raised two related design questions:
- Should we build a generic augmentation / metadata language into change
URIs, usable by these fakes and similar tooling — rather than a
fake-specific marker convention?
- The marker lookup (
TokenInChanges) arguably belongs on the Change API
(entity.Change) rather than living in a test-only helper.
This issue captures that idea as a follow-up so #197 can land as-is (the marker
stays test-only there).
Design considerations
A generic change-URI metadata facility would need to reconcile with the
strict provider change-ID parsers, which today reject anything that isn't a
canonical resource identifier:
submitqueue/entity/github/change_id.go — requires a full 40-char lowercase
hex head SHA; a ?key=value query string would not parse.submitqueue/entity/phabricator/change_id.go — fixed phab:// form.submitqueue/extension/changeprovider/github/validate.go — rejects
unexpected schemes.
Open questions:
- Scope — is URI-attached metadata a production capability, or strictly a
test/example affordance? Putting parsing on entity.Change couples the
production type to the convention; keeping it in core/fakemarker keeps the
separation the fakes deliberately maintain ("never production").
- Shape — RFC-3986 query params (
?k=v) vs. fragment vs. a dedicated
sidecar field on Change. Query params collide with the strict parsers
above; a typed field avoids string-encoding entirely.
- Generality — a neutral
key → value accessor (e.g.
Change.URIParam(key)) that fakemarker layers sq-fake semantics on top
of, vs. a richer metadata model.
References
Filed as a follow-up per the #197 review discussion. Not blocking.
Context
In #197, the extension fakes inject failures via an ad-hoc
sq-fake=<token>marker embedded in a change URI, parsed by the test-only
core/fakemarkerpackage (
Token([]string)/TokenInChanges([]entity.Change)).Reviewing that PR, @sbalabanov raised two related design questions:
URIs, usable by these fakes and similar tooling — rather than a
fake-specific marker convention?
TokenInChanges) arguably belongs on the Change API(
entity.Change) rather than living in a test-only helper.This issue captures that idea as a follow-up so #197 can land as-is (the marker
stays test-only there).
Design considerations
A generic change-URI metadata facility would need to reconcile with the
strict provider change-ID parsers, which today reject anything that isn't a
canonical resource identifier:
submitqueue/entity/github/change_id.go— requires a full 40-char lowercasehex head SHA; a
?key=valuequery string would not parse.submitqueue/entity/phabricator/change_id.go— fixedphab://form.submitqueue/extension/changeprovider/github/validate.go— rejectsunexpected schemes.
Open questions:
test/example affordance? Putting parsing on
entity.Changecouples theproduction type to the convention; keeping it in
core/fakemarkerkeeps theseparation the fakes deliberately maintain ("never production").
?k=v) vs. fragment vs. a dedicatedsidecar field on
Change. Query params collide with the strict parsersabove; a typed field avoids string-encoding entirely.
key → valueaccessor (e.g.Change.URIParam(key)) thatfakemarkerlayerssq-fakesemantics on topof, vs. a richer metadata model.
References
core/fakemarker.godiscussion in feat(extensions): fake implementations with error injection #197.Filed as a follow-up per the #197 review discussion. Not blocking.