From c979ec635367556c54513b7b293eda02307eb64e Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:53:39 +0100 Subject: [PATCH] docs: adapt the comment style guide to this repo, and sweep The guide cited files, documents and command packages from another repository, so two of its sections described a layout this tree does not have. Its rules are unchanged; the examples, the cross-reference targets and the per-package guidance now name what is here, and the transform example is a comment from this sweep. The sweep drops what the rules forbid and keeps what they are for. Every narrative sentence carried a fact, and the fact stays: the packages an SDK puts an inline request body in, the three ways a generated indexer spells a hop, why an envelope cannot be taken as the element. What goes is the account of how each was found, the counts measured on one run, and the asides. No behaviour change. unsupported.json is byte-identical on all three pilots and every tree reproduces without drift. Co-Authored-By: Claude Opus 5 (1M context) --- docs/comment-style.md | 129 ++++++++++-------- internal/config/reference.go | 6 +- internal/config/secrets.go | 6 +- internal/config/validate.go | 2 +- internal/corpus/corpus.go | 11 +- internal/emit/render_mapping.go | 9 +- internal/emit/render_schema.go | 19 ++- internal/emit/schema_type.go | 12 +- internal/emit/services.go | 17 ++- internal/emit/unsupported.go | 9 +- .../intermediate_representation/derive.go | 13 +- internal/intermediate_representation/model.go | 9 +- internal/providergen/providergen.go | 10 +- internal/sdkbind/loader.go | 17 ++- internal/sdkbind/prune.go | 5 +- internal/sdkbind/prune_calls.go | 5 +- internal/spec/revise/report.go | 10 +- internal/spec/revise/revise.go | 4 +- 18 files changed, 142 insertions(+), 151 deletions(-) diff --git a/docs/comment-style.md b/docs/comment-style.md index 4f964f3..83a4565 100644 --- a/docs/comment-style.md +++ b/docs/comment-style.md @@ -2,11 +2,15 @@ Comments in this repo state **what** and **why**. Nothing else. -They are not a record of how the code was arrived at. The investigation belongs in -`docs/backends.md` and in commit messages, which are where this project already -keeps measured evidence. A comment that retells the debugging story buries the one -or two sentences a reader needs, and it ages badly — the narrative stays after the -finding it describes has been superseded. +They are not a record of how the code was arrived at. The investigation belongs +in the pull request body and in commit messages, which are where this project +keeps measured evidence and where a measurement carries a date. A comment that +retells the debugging story buries the one or two sentences a reader needs, and +it ages badly — the narrative stays after the finding it describes has been +superseded. + +`CLAUDE.md` states the same rule in short form; this document is the long form, +and the two do not disagree. ## The rule @@ -31,76 +35,81 @@ statement whose reason is local to that statement. These are facts, and they are the point of the comment. -- **External-system behaviour, as a standing property.** "HNS reads - `Ipams[].Subnets[]` back as a flat `Subnets[]`." "`query user` answers - *No User exists for \** on a guest that plainly has a console session." -- **Accepted and rejected value sets**, and the hardware or build-number - qualifiers that scope a claim: "SecureNestedPaging requires AMD SEV-SNP", - "measured on build 26200". -- **Raw evidence quoted inline** — HRESULTs (`0x8007139F`), SDDL strings - (`D:P(A;;FA;;;BA)(A;;FA;;;SY)`), Winsock error names. They are what makes the - surrounding claim checkable. +- **Generator and API behaviour, as a standing property.** "kiota models an + object union as a composed type with one named accessor per branch." "A + document that declares nothing required in its responses sends every writable + attribute to plain `Optional`." +- **Accepted and rejected value sets**, and the backend or version qualifiers + that scope a claim: "`list/schema` attribute types declare no `Sensitive` + field", "measured against terraform-plugin-framework v1.19.0". +- **Raw evidence quoted inline** — an SDK accessor spelling + (`GetPasswordEscaped`), a refusal reason as the report prints it, an HTTP + status, a framework type name. They are what makes the surrounding claim + checkable. - **Indented tabular blocks.** They render as godoc code blocks and usually carry - more than the prose around them. See `internal/hcs/netmodes.go` (mode → HCN - network) and `internal/hcs/reconcile.go` (network type → how it reports - `SwitchGuid`). Keep the table, rewrite the prose framing it. -- **Cross-references.** Prefer paths and symbols over URLs: `docs/backends.md`, - gate ids (G1, G2, G4), milestone ids (M3, M4), env var names. Keep a gate or - milestone id next to its `docs/backends.md` reference so it stays resolvable. - This is a preference about *form*, not a licence to delete citations — a - pointer into upstream source that lets a reader check a claim - (`hcsshim internal/uvm/create_wcow.go`) is evidence and stays. Drop a bare URL - only when the sentence still stands without it. + more than the prose around them. See `internal/corpus/cache.go` (the cache + layout) and `internal/sdkbind/binder_kiota.go` (document shape → builder + chain). Keep the table, rewrite the prose framing it. +- **Cross-references.** Prefer paths and symbols over URLs: `docs/contract.md`, + `docs/glossary.md`, `docs/mapping.md`, `x-tfpfgen-*` keys, `TFPFGEN_*` + environment variable names, the exit-code contract. Keep a mapping.md row + number next to its reference so it stays resolvable. This is a preference + about *form*, not a licence to delete citations — a pointer into upstream + source that lets a reader check a claim + (`terraform-plugin-framework-validators stringvalidator/utf8_length_between.go`) + is evidence and stays. Drop a bare URL only when the sentence still stands + without it. - **Contracts** — locking requirements, `nil, nil` returns, ordering guarantees, - what a caller must do first. -- **Operator-visible risk in acceptance tests** — why a test is opt-in, what it - does to the host. State it as a standing risk, not as an anecdote. + what a caller must do first, which stage owns a refusal. +- **Operator-visible risk in credentialed tests** — why a test is opt-in, what it + does to somebody's tenant. State it as a standing risk, not as an anecdote. ## Drop - **Discovery narrative and post-mortems.** "used to", "we tried", "turned out", "it was tried and does NOT work", "that happened and nothing noticed", - "for as long as weave asked HNS to…", "which is how X became Y". -- **Incident statistics and blame.** "one exec in fifteen hundred bound the module - bus's port", "it cost an acceptance run", "this gate has already produced two - 'we tested that' claims that turned out to be testing something else". + "which is how X became Y", "had already drifted into a duplicate". +- **Counts and measurements from a particular run.** "207 losses on one pilot", + "the first live run opened fifty-seven", "42 attributes across the three + pilots". A measurement belongs in a pull request body, where it is dated. - **Retracted conclusions.** State only what is true now. A superseded finding kept "because it was load-bearing for a while" is a trap for the next reader. -- **Roadmap and project commentary in API docs.** Which milestone a refactor lands +- **Roadmap and project commentary in API docs.** Which tranche a refactor lands in is not a property of the function. -- **Editorial asides.** "the crux of the Windows guest story", "worth having - anyway", "the rest is left alone deliberately". -- **Restatements of the signature.** `// New returns an HCS engine.`, - `// Resume resumes a paused VM.`, `// Profiles returns the registry.` Either say - something the signature does not, or say nothing. -- **Porting history**, beyond a single attribution line in a file header. +- **Editorial asides.** "worth having anyway", "the rest is left alone + deliberately", "which is the point". +- **Restatements of the signature.** `// Prune prunes the bindings.`, + `// Load loads a document.`, `// IDs returns the ids.` Either say something the + signature does not, or say nothing. +- **Which pull request changed it**, and how a bug was found. Git history + records change; a comment states what is true. ## Transform, don't delete Almost every narrative sentence contains a fact. Convert the story into the standing property it implies; do not throw the fact out with it. -Before — 14 lines: +Before — 11 lines: -> installedMatches reports whether the guest already holds exactly this binary, -> comparing content hashes. +> joinTreeKeeping is joinTree, and also answers the attributes it kept with no +> SDK field behind them — the id and the addressing attributes. > -> The obvious check — ask the deployed agent its version and compare — is not -> enough, and the way it fails is quiet. The version identifies the protocol, so -> it does not move when the binary changes for any other reason: a bug fix, a -> dependency bump, a different link flag. Rebuilding the agent for the GUI -> subsystem changed nothing about the protocol, so a version check reported the -> guest up to date and left the old binary running, console window and all. +> Pruning removes their bindings, correctly: no model carries them, because they +> address the object rather than describe it. But the attribute still reaches the +> schema, so reporting that removal as something the operator lost is wrong, and +> it was wrong 207 times on one pilot. This is the only place that knows, because +> this is the place that decides. -After — 6 lines, same information, no story: +After — 7 lines, same information, no run-specific count and no editorial: ```go -// installedMatches reports whether the guest already holds exactly this binary, -// comparing content hashes. +// joinTreeKeeping is joinTree, and also answers the attributes it kept with +// no SDK field behind them — the id and the addressing attributes. // -// A version comparison is not sufficient: the version identifies the protocol, -// so it does not move when the binary changes for any other reason — a bug fix, -// a dependency bump, or a link flag such as -H windowsgui. +// Pruning removes their bindings correctly: no model carries them, because +// they address the object rather than describe it. The attribute still +// reaches the schema, so reporting that removal as a loss would be wrong, +// and this is the only place that knows which attributes those are. ``` ## Section dividers @@ -134,12 +143,12 @@ package hcs ## Where the doc goes for commands -- **`internal/command`** — the `XxxCommand` struct carries the prose that explains - the feature. Its `Validate` and `Run` methods each carry one to three lines - saying what they do and any state precondition they enforce. - **`internal/cli`** — user-facing text lives in the cobra `Short`, `Long` and `Example` fields, which is the right home for it. Each `newXxxCommand` - constructor still carries a one-line Go doc so `go doc` is not blank. -- **`cmd/hcsspike`** — a lab notebook rather than a library. Its measured verdict - tables and pass/fail matrices are the deliverable and stay verbatim. The - self-narration around them does not. \ No newline at end of file + constructor still carries a one-line Go doc so `go doc` is not blank. A verb's + exit-code behaviour belongs here, next to `docs/contract.md`'s table. +- **`cmd/tfpfgen`** — three lines of substance over `cli.Run`. It needs a package + comment and nothing else; everything a reader wants is in `internal/cli`. +- **`internal/templates`** — a template's comments are emitted into somebody + else's repository, where nobody can edit them. They say what the generated + code does, never how this toolkit decided to generate it. \ No newline at end of file diff --git a/internal/config/reference.go b/internal/config/reference.go index ca985a6..10baa36 100644 --- a/internal/config/reference.go +++ b/internal/config/reference.go @@ -2,9 +2,9 @@ package config // docs/config.md is generated, not written: Reference walks the Config // struct by reflection, so the reference can only describe keys the decoder -// actually reads. v1 documented keys that nothing consumed; here a dead key -// in the docs is structurally impossible, and the bidirectional test over -// the descriptions map makes a dead description impossible too. +// actually reads: a documented key nothing consumes is structurally +// impossible, and the bidirectional test over the descriptions map makes a +// description with no key impossible too. import ( "fmt" diff --git a/internal/config/secrets.go b/internal/config/secrets.go index 41ed73c..8bc1f71 100644 --- a/internal/config/secrets.go +++ b/internal/config/secrets.go @@ -1,9 +1,9 @@ package config // Secret names are fixed by auth role — they are a contract, not -// configuration. v1 let the workflow and the config each spell secret names -// and a rename broke the pipeline with an error citing the old spelling; -// here there is exactly one definition site. +// configuration, and this is their one definition site. Letting the workflow +// and the config each spell them means a rename breaks the pipeline with an +// error citing the spelling that was replaced. const ( SecretToken = "TFPFGEN_AUTH_TOKEN" SecretClientID = "TFPFGEN_AUTH_CLIENT_ID" diff --git a/internal/config/validate.go b/internal/config/validate.go index 78ea0b3..50e06e6 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -15,7 +15,7 @@ var dnsLabel = regexp.MustCompile(`^[a-z][a-z0-9-]*[a-z0-9]$`) // releaseTag matches an exact toolkit release. Branch names — "main" // included — are deliberately unmatchable: a pipeline that reinstalls a -// moving target on every run was one of v1's failure modes. +// moving target on every run cannot reproduce what it generated. var releaseTag = regexp.MustCompile(`^v\d+\.\d+\.\d+$`) // problems returns every semantic defect at once. An empty slice means the diff --git a/internal/corpus/corpus.go b/internal/corpus/corpus.go index a1d3828..0aee0c0 100644 --- a/internal/corpus/corpus.go +++ b/internal/corpus/corpus.go @@ -55,9 +55,8 @@ const EnvRequired = "TFPFGEN_CORPUS_REQUIRED" // relative default is resolved against the working directory, and `go test` // runs every package in its own directory -- so one suite run would scatter a // copy of every document through the tree, in package directories no -// root-anchored .gitignore pattern reaches. v1 learned this the hard way: it -// put 19 MB of fetched specifications into a commit whose entire purpose was -// removing generated output from the repository. +// root-anchored .gitignore pattern reaches, and the documents are megabytes +// each. // // A cache belongs where the operating system puts caches. CI overrides it with // TFPFGEN_CORPUS_DIR to somewhere its cache action can restore. @@ -92,9 +91,9 @@ type Pin struct { // MirrorURL holds the exact pinned bytes under our own control, and is // tried first. // - // Not a nicety. Several of these upstreams serve "current" rather than a - // version, so the pinned bytes become unfetchable the moment the vendor - // publishes again, and every pin update is forced on their schedule. A + // Several of these upstreams serve "current" rather than a version, so + // the pinned bytes become unfetchable the moment the vendor publishes + // again, and every pin update is forced on their schedule. A // mirror cannot alter test meaning -- the SHA-256 is checked identically // whichever source answered -- so it is a CDN, not a source of truth. MirrorURL string `json:"mirrorUrl,omitempty"` diff --git a/internal/emit/render_mapping.go b/internal/emit/render_mapping.go index 6daf6e0..7336d56 100644 --- a/internal/emit/render_mapping.go +++ b/internal/emit/render_mapping.go @@ -364,11 +364,10 @@ func buildCallPlan(call *sdkbind.Call, payloadName string, nodes []node, modelVa for position, p := range call.Params { // The last path parameter addresses the object itself, which is what // the id attribute holds however the API spells the parameter. The - // fallback used to need the call to take exactly one parameter, which - // is only true of a flat API: /enterprises/{enterprise}/code-security/ - // configurations/{configuration_id} takes two, and its id was left - // matching nothing because the response happened to declare an id of - // its own and keep that spelling. + // fallback is offered on position rather than on the call taking + // exactly one parameter: only a flat API is single-parameter, and + // /enterprises/{enterprise}/code-security/configurations/{configuration_id} + // takes two while still naming one object. n, err := paramNode(p, nodes, position == len(call.Params)-1) if err != nil { return callPlan{}, err diff --git a/internal/emit/render_schema.go b/internal/emit/render_schema.go index 935fea2..33b3beb 100644 --- a/internal/emit/render_schema.go +++ b/internal/emit/render_schema.go @@ -285,9 +285,8 @@ func (sb *schemaBuilder) planModifierLines(n node, indent string) string { // The order is the point. The document's sentence is the only human-written // text in the whole pipeline and it is what a practitioner actually needs; // the inferred facts qualify it. Where the document says nothing — and real -// ones often do not, one pilot annotating 12% of its properties against -// another's 52% — the wire property name stands in, which is no worse than -// what was rendered before and no better. +// ones routinely annotate only a fraction of their properties — the wire +// property name stands in. func attributeDescription(a ir.Attribute) string { var parts []string if declared := strings.TrimSpace(a.Description); declared != "" { @@ -329,14 +328,12 @@ type modelDecl struct { // // The short spelling — the type prefix, the attribute's Go name, "Model" — // is what a tree gets whenever it is the only nesting site claiming it. Two -// sites claiming one spelling used to be a hard error naming both paths and -// telling the operator to "rename one in the document", which is not -// something a vendor's document will do: a real document nests an object of -// one name at two depths of one entity, and that single collision aborted -// every resource the provider had. Every claimant of a contested spelling is -// therefore qualified by its ancestor path instead, and an uncontested one is -// left exactly as it was — so qualification shows up only where a collision -// made it necessary. +// sites claiming one spelling cannot be refused: a real document nests an +// object of one name at two depths of one entity, and a vendor will not +// rename it to suit a generator. Every claimant of a contested spelling is +// qualified by its ancestor path instead, and an uncontested one is left +// short — so qualification shows up only where a collision made it +// necessary. // // Which sites are contested is decided from the whole tree before any name // is handed out, so the answer never depends on the order names are asked diff --git a/internal/emit/schema_type.go b/internal/emit/schema_type.go index 2a8de95..752b106 100644 --- a/internal/emit/schema_type.go +++ b/internal/emit/schema_type.go @@ -11,14 +11,10 @@ import ( // that follows from "this attribute is a list of objects" or "this // attribute is a bool", resolved once and read as fields. // -// It exists because those names used to live in six switches on the same -// (Kind, ElementType, Nested != nil) triple, spread across four files, each -// returning a bare string. Nothing tied them together, so adding an -// attribute type meant editing six places and hoping none was missed, and -// the compiler could not help — every one of them returned string. One of -// the six had already drifted into a duplicate: frameworkValueType in -// render_datasource.go answered exactly what fieldType answered for a -// scalar, spelled separately. +// One record rather than a switch per name: every one of these follows from +// the same (Kind, ElementType, Nested != nil) triple, and answering them +// separately means a new attribute type has to be added everywhere at once, +// with the compiler unable to help because each answer is a bare string. // // Field names are terraform-plugin-codegen-spec's, so this record and the // specification that could describe its output agree: ValueType from diff --git a/internal/emit/services.go b/internal/emit/services.go index 5084bbf..1455320 100644 --- a/internal/emit/services.go +++ b/internal/emit/services.go @@ -320,10 +320,10 @@ func rawFile(outPath, source string, content []byte) File { // requires every resource and datasource to have an id; the API need not // agree. A read response that names its key something other than id, or omits // it altogether and leaves it in the URL, is an ordinary REST shape, and both -// are common in real documents. Dropping the id node on those used to abort -// the whole provider from render_mapping's path-parameter lookup. The id's -// value comes from the path parameter and from whatever the create response -// carries, neither of which needs a read binding. +// are common in real documents. The id node has to survive them both: +// render_mapping's path-parameter lookup reads it, and dropping it aborts the +// whole provider. Its value comes from the path parameter and from whatever +// the create response carries, neither of which needs a read binding. type node struct { attr ir.Attribute fb *sdkbind.FieldBinding @@ -341,11 +341,10 @@ func joinTree(tree *ir.AttributeTree, fbs []sdkbind.FieldBinding, addressing ... // joinTreeKeeping is joinTree, and also answers the attributes it kept // with no SDK field behind them — the id and the addressing attributes. // -// Pruning removes their bindings, correctly: no model carries them, because -// they address the object rather than describe it. But the attribute still -// reaches the schema, so reporting that removal as something the operator -// lost is wrong, and it was wrong 207 times on one pilot. This is the only -// place that knows, because this is the place that decides. +// Pruning removes their bindings correctly: no model carries them, because +// they address the object rather than describe it. The attribute still +// reaches the schema, so reporting that removal as a loss would be wrong, +// and this is the only place that knows which attributes those are. func joinTreeKeeping(tree *ir.AttributeTree, fbs []sdkbind.FieldBinding, addressing ...map[string]bool) ([]node, []string) { names := map[string]bool{idAttributeName: true} for _, set := range addressing { diff --git a/internal/emit/unsupported.go b/internal/emit/unsupported.go index 8905417..b5b7336 100644 --- a/internal/emit/unsupported.go +++ b/internal/emit/unsupported.go @@ -100,11 +100,10 @@ func RenderUnsupported(m *ir.Model, removals []sdkbind.Removal, dropped []sdkbin } for _, removal := range removals { - // A removal the emitter kept anyway cost the operator nothing: the - // binding went because no model carries the field, and the - // attribute reached the schema regardless. Reporting it as a - // refusal is how this file came to claim 207 losses on one pilot - // that were not losses at all. + // A removal the emitter kept anyway costs the operator nothing: the + // binding goes because no model carries the field, and the + // attribute reaches the schema regardless. Reporting it as a + // refusal overstates what the provider will not carry. if keptUnbound[keptUnboundKey(removal.Kind, removal.Key, removal.Attribute)] { continue } diff --git a/internal/intermediate_representation/derive.go b/internal/intermediate_representation/derive.go index 8d3baac..7143a1c 100644 --- a/internal/intermediate_representation/derive.go +++ b/internal/intermediate_representation/derive.go @@ -352,13 +352,12 @@ func listEnvelopeKey(list *specmodel.Operation) string { // x-tfpfgen-list-response-shape where the audit recorded one and from the // document's own single array property otherwise. // -// The envelope used to be taken as-is, which made the element tree the -// envelope's own fields. Nothing then bound: the SDK reaches through the -// envelope to the element, so derivation asked for results and totalCount -// off a model carrying id, username and date, every attribute was pruned, -// and the entity was removed for having nothing left to map. Envelopes are -// not the exception — one pilot wraps 166 of its collection responses and -// leaves only 36 bare. +// Taking the envelope as-is would make the element tree the envelope's own +// fields, and nothing would bind: the SDK reaches through the envelope to the +// element, so derivation would ask for results and totalCount off a model +// carrying id, username and date, every attribute would prune, and the entity +// would be removed for having nothing left to map. Envelopes are the common +// shape rather than the exception. // // A response that is neither is returned unchanged: it may be a single // object the classification took for a collection, and guessing an element diff --git a/internal/intermediate_representation/model.go b/internal/intermediate_representation/model.go index 862f63c..cbd7353 100644 --- a/internal/intermediate_representation/model.go +++ b/internal/intermediate_representation/model.go @@ -6,11 +6,10 @@ // The name is owner-mandated, underscores included, and the package is // ephemeral by design. Derive is a pure function of the revised document // and tfpfgen.yaml, recomputed on every generation run, and the result is -// never written to the repository. v1's fatal flaw was a committed -// intermediate representation: the moment the derived file lived in git it -// became a second source of truth, hand adjustments to it fought every -// regeneration, and the document stopped being what generation actually -// read. Here the revised document is the only committed truth; anything a +// never written to the repository. A committed intermediate representation +// is a second source of truth: hand adjustments to the derived file fight +// every regeneration, and the document stops being what generation actually +// reads. Here the revised document is the only committed truth; anything a // human wants changed goes through a correction to that document, and the // derivation follows. // diff --git a/internal/providergen/providergen.go b/internal/providergen/providergen.go index bdab7d4..89c9a37 100644 --- a/internal/providergen/providergen.go +++ b/internal/providergen/providergen.go @@ -280,11 +280,11 @@ func generate(opts Options) (*generation, error) { return nil, err } // A binding the SDK cannot answer is a fact about one entity, not a - // defect in the toolkit, and it used to take the whole provider with it: - // one entity naming a read model the backend never emitted was enough to - // stop every other entity in a large document. The entity is dropped from - // the bindings — which is exactly what emission already does with an - // entity the bindings lack — and reported as excluded. + // defect in the toolkit, so it must not stop the run: one entity naming + // a read model the backend never emitted would otherwise take every + // other entity in the document with it. The entity is dropped from the + // bindings — which is exactly what emission already does with an entity + // the bindings lack — and reported as excluded. excluded := bindings.DropProblems(rep.Problems) core, err := emit.RenderProviderCore(pc) diff --git a/internal/sdkbind/loader.go b/internal/sdkbind/loader.go index 07b0aa4..82d05f8 100644 --- a/internal/sdkbind/loader.go +++ b/internal/sdkbind/loader.go @@ -262,15 +262,14 @@ func (l *loader) typeFromExpr(info SDKInfo, expr string) (*types.Named, error) { // typeAndPackageFromExpr resolves a type expression and answers the import // path it was found under, so a caller that has to import the package can. // -// Two package qualifiers are known by construction: models, and the SDK root -// everything else used to fall back to. That fallback was wrong for a whole -// class of type. A generator puts the model for an inline request body — one -// the document declares in place rather than naming under components — in the -// package of the operation that takes it, not in models: a body declared -// inline on POST /orgs/{org}/teams becomes orgs.ItemTeamsPostRequestBody. -// Looking those up in the SDK root failed with "package sdk has no type -// ItemTeamsPostRequestBodyable (available: APIClient)", and took the resource -// with it — every resource of a document that declares its bodies inline. +// Only one package qualifier is known by construction: models. The SDK root +// is not a safe fallback for the rest, because a generator puts the model for +// an inline request body — one the document declares in place rather than +// naming under components — in the package of the operation that takes it: a +// body declared inline on POST /orgs/{org}/teams becomes +// orgs.ItemTeamsPostRequestBody. Looking one up in the SDK root fails with +// "package sdk has no type ItemTeamsPostRequestBodyable (available: +// APIClient)", and takes the whole resource with it. // // An unknown qualifier is therefore resolved by searching the loaded // packages for one of that name that declares the type. Exactly one match diff --git a/internal/sdkbind/prune.go b/internal/sdkbind/prune.go index bb5a20c..851304b 100644 --- a/internal/sdkbind/prune.go +++ b/internal/sdkbind/prune.go @@ -178,9 +178,8 @@ func (p *pruner) resource(rb *ResourceBinding) bool { // where an API declares them separately the generated SDK types them // separately: passing the create body to the update does not compile. The // update's fields are the same attributes resolved against the update's own -// model, so whatever it cannot carry is dropped from the update alone — -// which is the point, since the difference between the two bodies is -// usually exactly a handful of fields. +// model, so whatever it cannot carry is dropped from the update alone. The +// two bodies usually differ by only a handful of fields. // // A resource whose update body cannot be constructed keeps its create-only // body rather than being removed: the update will not compile against it, diff --git a/internal/sdkbind/prune_calls.go b/internal/sdkbind/prune_calls.go index 0df5e15..4b00b42 100644 --- a/internal/sdkbind/prune_calls.go +++ b/internal/sdkbind/prune_calls.go @@ -269,9 +269,8 @@ const indexerPrefix = "By" // the draft asked for ByGistId; it appends a suffix where the parameter is // bare, so owner becomes ByOwnerId; and where it has already used a name it // renames wholesale, so team_slug becomes ByEnterpriseTeamId. Three -// different rules, none of them derivable from the document — and between -// them they took thirty-eight of one document's forty resources, every one -// with a read call the SDK plainly had. +// different rules, none of them derivable from the document, and each one +// enough to refuse a resource whose read call the SDK plainly has. // // A collection builder indexes by exactly one thing, so where it declares // exactly one such method there is nothing to guess: the hop is that method diff --git a/internal/spec/revise/report.go b/internal/spec/revise/report.go index ed10eb1..f5da9a1 100644 --- a/internal/spec/revise/report.go +++ b/internal/spec/revise/report.go @@ -4,12 +4,10 @@ // // It exists because the correction files themselves cannot narrate. A // correction is a justification, some RFC 6902 operations and a pointer at an -// observation — enough to apply, nowhere near enough to review. The job that -// opens pull requests was reading exactly those files and could therefore -// only quote them, which is how a reviewer ended up with one pull request per -// attribute, each saying nothing about what was actually asked of the API or -// what came back. The report is additional: the correction files are written -// exactly as before. +// observation — enough to apply, nowhere near enough to review. A job that +// reads only those files can only quote them, and a reviewer needs what was +// asked of the API and what came back. The report is additional: the +// correction files are written exactly as before. package revise import ( diff --git a/internal/spec/revise/revise.go b/internal/spec/revise/revise.go index 093581e..c3f8f4e 100644 --- a/internal/spec/revise/revise.go +++ b/internal/spec/revise/revise.go @@ -95,8 +95,8 @@ func Materialize(dir string) (Result, error) { // refusePending fails while any proposed correction awaits a decision, // naming each file and the two legal resolutions. No flag bypasses this: -// v1's -allow-conflicts taught that an ignorable gate is an ignored gate, -// so the only ways forward are the decisions themselves. +// an ignorable gate is an ignored gate, so the only ways forward are the +// decisions themselves. func refusePending(correctionsDir string) error { proposedDir := filepath.Join(correctionsDir, ProposedDirName) entries, err := os.ReadDir(proposedDir)