Skip to content

Decompose GitHub resources into products - #10

Merged
coenttb merged 6 commits into
mainfrom
issue-7-resource-decomposition
Aug 3, 2026
Merged

Decompose GitHub resources into products#10
coenttb merged 6 commits into
mainfrom
issue-7-resource-decomposition

Conversation

@coenttb

@coenttb coenttb commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Implements the per-resource product/target decomposition from Task #7 as a one-commit stack on PR #9 at its exact independently reviewed head 0f49fa7f4a4ca30cde29d81f5c7b7522574cddac.

  • exposes OAuth, Organizations, Repositories, Stargazers, Traffic, and Users as independent library products/targets while retaining GitHub as the aggregate;
  • moves all production source blobs unchanged into their owning resource targets and declares Async/Either only on the C5 traversal modules that use them;
  • splits the former core test target into six resource test targets, with target-local fixtures where the old suite shared internal test declarations.

Verification at 7fb24ee40ce83ed5ed0ec549cf74f08ba0016176:

  • workspace package test --package-path . --fresh --jobs 4 passed all 17 tests with Apple Swift 6.4;
  • Swift 6.3.3 swift-format lint --strict --ignore-unparsable-files passed across the complete changed Swift surface;
  • workspace package lint --package-path . exited 0 with 37 advisory findings.

This PR intentionally targets issue-6-c5, so its review diff contains only Task #7. Landing remains gated on PR #9 landing first and Task #7 obtaining its own qualifying evidence; this PR does not close Task #7.

@coenttb coenttb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING — exact-head review of 7fb24ee40ce83ed5ed0ec549cf74f08ba0016176 against stacked base 0f49fa7f4a4ca30cde29d81f5c7b7522574cddac.

Checked evidence: PR #10 is public, OPEN, non-draft, and one commit whose parent and merge-base are the exact PR #9 head, so the 64-path diff is Task #7-only. I read the complete diff, all 36 content-identical source moves, the manifest, exports, resource source, and localized tests. The OAuth, Organizations, Repositories, Stargazers, Traffic, and Users seams, their production dependency edges, and the aggregate GitHub re-export are coherent. Task #7 remains OPEN, typed Task, assigned to the source owner, and routed to this exact-owner repository.

Changed-surface blocker: GitHub.OAuth.Client Tests.swift directly imports RFC_3986, but the newly declared GitHub OAuth Core Tests target depends only on GitHub OAuth, and the package manifest does not declare swift-rfc-3986. This relies on transitive visibility through GitHub Standard; every product a target source imports must be a direct target dependency. The smallest fix is for the Task #7 source owner to add the canonical https://github.com/swift-ietf/swift-rfc-3986.git package dependency at the same main branch and .product(name: "RFC 3986", package: "swift-rfc-3986") to GitHub OAuth Core Tests, or remove the direct module import/use through an equally direct lawful surface. Push a new head and return it for exact-head re-review.

Task-attributed evidence at this head remains useful but does not discharge the manifest defect: fresh capped Workspace package tests passed 17 tests across the six resource test targets on Apple Swift 6.4; Swift 6.3.3 strict formatting passed; package lint exited 0; and the manifest evaluation reported seven products.

Next owner: the assigned Task #7 source owner. Landing remains serialized behind PR #9 landing and Task #7 qualifying evidence after this blocker is fixed. I did not edit source, dispatch or wait on CI, consume a Workspace build lane, merge, close or unassign Task #7, or alter PR #9.

@coenttb coenttb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLEAN — exact-head re-review of 8976310187809b1b418f98c4fe01847bedb4e04c against stacked base 0f49fa7f4a4ca30cde29d81f5c7b7522574cddac.

Checked evidence: PR #10 is public, OPEN, non-draft, and remains stacked on the exact PR #9 head. The replacement commit is the sole direct child of the previously reviewed head and changes only Package.swift: one canonical https://github.com/swift-ietf/swift-rfc-3986.git dependency on main, plus .product(name: "RFC 3986", package: "swift-rfc-3986") on GitHub OAuth Core Tests. The complete head manifest has four distinct canonical package identities, so the replacement introduces neither a duplicate identity nor an alternate spelling. The upstream swift-rfc-3986 manifest at 690e453 exports that exact product from its current default main branch.

The complete affected-target import context confirms the direct edge is necessary and correctly attached: the client test at this head imports RFC_3986 and uses both RFC_3986.Error and RFC_3986.URI; its two sibling fixture files import only GitHub_OAuth. Task #7 remains OPEN, typed Task, assigned to the source owner, and routed to the exact owning repository.

Prior-reviewed evidence reused with provenance: review #4838469221, bound to parent 7fb24ee40ce83ed5ed0ec549cf74f08ba0016176, read the complete 64-path Task #7 diff and found the resource seams, source moves, target-local tests, production dependency edges, and aggregate re-export coherent; its sole blocker was this missing direct test dependency. Because the replacement is exactly the one-file direct child above, those clean parent findings remain applicable. The source-owner receipt at this replacement head reports manifest evaluation, strict formatting and diff checks, and fresh package evidence; I treat those as task-attributed and did not relabel them as independently originated.

Verdict: the sole direct-dependency blocker is resolved, and no replacement defect exists. Landing remains serialized behind Task #6 / PR #9 landing and Task #7 qualifying evidence. Next owner is the landing coordinator after those prerequisites; this review does not merge, alter PR #9, or change Task #7 state.

Base automatically changed from issue-6-c5 to main August 2, 2026 13:00
@coenttb

coenttb commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator Author

Temporarily closing this pull request before updating its branch ancestry so the obsolete pull-request-triggered full development matrix is not started. The ancestry-only merge will preserve the exact reviewed Task #7 tree at 8976310. This pull request will remain closed until swift-institute/.github#186 lands, then it can be reopened against main.

@coenttb coenttb closed this Aug 2, 2026
@coenttb coenttb reopened this Aug 3, 2026
@coenttb

coenttb commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Reopened per Task #7 after the swift-institute/.github#186 blocker cleared (merged 2026-08-02T14:31Z).

Branch ancestry was already updated onto main prior to this reopen — merge commit cc226a1 ("Merge main into Task #7 branch") merges current main (c27af1c, the squash-landed Task #6 content) into issue-7-resource-decomposition. origin/main is confirmed an ancestor of the branch head; no further merge or conflict resolution was needed. Local sanity build (workspace package build --jobs 4) passed clean at this head.

@coenttb coenttb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review — head cc226a16c0acdd20d213c739287a01f9d2481862, base main @ c27af1c1ab768fbc191f3bb6431c3bc5184aa27d.

Verdict: one blocker, in documentation only. The decomposition itself is correct and needs no rework.

Blocker

README.md lines 86-87 contradict the manifest this PR ships.

No deprecated GitHub Traffic, GitHub Stargazers, or GitHub Repositories compatibility products are provided.

At this head Package.swift declares library products named exactly GitHub Traffic, GitHub Stargazers, and GitHub Repositories. The sentence was true at the base commit and is false at this head, and this PR is what made it false. On a public repository the README is the front door, so this would ship a direct README-versus-manifest self-contradiction in the same commit that creates the products.

Two edits are needed:

  1. Replace README.md:86-87. Those names are now first-class per-resource products, not deprecated compatibility shims.
  2. README.md:32-39 documents only .product(name: "GitHub", package: "swift-github"). Per-resource products as a supported consumer choice are the outcome Task #7 asks for; a consumer reading this head cannot discover that the six exist. Add them to Installation.

The drift is invisible to CI by construction: Tests/GitHub Tests/ReadmeVerificationTests.swift is the file that would assert README fidelity, and it belongs to no declared test target, so it is never compiled or run. See adjacent debt.

Checked and clean

Decomposition. Six targets each own exactly one resource family and nothing else — OAuth, Organizations (/orgs/{org}/repos), Repositories (/repos/{owner}/{repo} and contents), Stargazers, Traffic, Users (/user, /user/emails, /user/repos). Each has its own test target, matching Task #7's stated outcome. Every production blob moved with a rename-only diff, so the split is auditable as pure relocation.

Dependency direction. Downward-only and acyclic: each leaf depends on GitHub Standard and, where used, the primitives; the GitHub aggregate depends on the six leaves; no leaf depends on the aggregate, there is no leaf-to-leaf edge, and no github.com/coenttb/* edge.

Boundaries chosen independently. Target boundaries moved; the namespace did not. All six modules continue to vend types under the single GitHub.* namespace (extension GitHub.Repository.Stargazers living in module GitHub_Stargazers). That is the right call, and it is why the split costs consumers nothing.

Composition — manifest edges match file-level imports exactly. Async Lifecycle Primitives and Either Primitives are declared on exactly GitHub Organizations, GitHub Stargazers, and GitHub Users — precisely the three targets whose sources reference Either / Async.Lifecycle, i.e. the C5 traversal modules. GitHub OAuth, GitHub Repositories, and GitHub Traffic declare GitHub Standard alone and use nothing else. No over-declaration and no under-declaration anywhere in the seven targets.

RFC 3986. Declaring swift-rfc-3986 for GitHub OAuth Core Tests corrects a previously implicit transitive import — that test's import RFC_3986 resolved only through the umbrella re-export chain. It costs no extra resolution, because swift-github-standard already depends on swift-rfc-3986 and it was in the closure already.

API surface — additive; no consumer coordination required. The GitHub product is retained under the same name, and its @_exported public import chain to the six leaves (each re-exporting GitHub Standard, plus the primitives on the traversal targets) reproduces the exact pre-PR transitive surface. I swept the declared consumers rather than inferring: every manifest that depends on this package takes .product(name: "GitHub", package: "swift-github") and nothing else, and every import site spells bare import GitHub. The downstream binding swift-github-http is representative — 15 source files, all import GitHub. No consumer names a resource module, so the new products are purely additive and no consumer-visible rename or move is introduced. That is consistent with the passing API-breakage check rather than merely asserted by it.

Typed throws and leaf errors. The ruled C5 shape is preserved. Splitting the test targets correctly forced resource-local failure leaves — GitHub.Repository.Stargazers.Fixture.Failure, GitHub.Repository.Traffic.Fixture.Failure, and GitHub.User.Fixture.Failure now serve call sites that previously borrowed GitHub.Repository.Fixture.Failure across what has become a target boundary. That is the ruling applied, not worked around.

Hygiene and freshness. Package.resolved is untracked. The base is exactly the current main, which is PR #9's merge commit, so there is no rebase drift.

Adjacent debt — pre-existing, not introduced here, do not fix in this PR

  • Sources/GitHub Shared/exports.swift is an undeclared orphan whose @_exported import names two modules (GitHub_Live_Shared, Dependencies) that this package does not depend on. This PR retired its twin — the former Sources/GitHub Repositories/exports.swift, which named the equally dead GitHub_Repositories_Live / GitHub_Shared — by promoting that directory into a real target. Retiring one of a pair and leaving the other is an incomplete sweep of a single class.
  • Tests/GitHub Tests/, Tests/GitHub Repositories Tests/, and Tests/GitHub Traffic Tests/ belong to no declared test target. Worth noting for the follow-up: Tests/GitHub Traffic Tests/GitHubTrafficTests.swift already opens with import GitHub_Traffic and Tests/GitHub Repositories Tests/GitHubRepositoriesTests.swift with import GitHub_Repositories, so this PR is what first makes those two modules exist — those directories become declarable for the first time here. Tests/GitHub Tests/ReadmeVerificationTests.swift is the opposite case: it references GitHub_Types_Shared, Dependencies, and a GitHub.Client type, none of which exist in this package, so it could not compile if declared and needs rewriting rather than wiring.
  • The PR description states this PR targets issue-6-c5. The live base is main, and PR #9 has merged as c27af1c1. Worth refreshing so the description matches the branch it will land from.

Evidence read, and what the full tier must still prove

PR-tier run 30790473082 at this head passed Ubuntu Swift 6.3 (release), Ubuntu Swift 6.4.x nightly (release), SwiftLint, swift-format lint, swift-linter, API breakage (γ-1c), PR title format, Plan, DocC archive, and ci-ok.

It skipped macOS, Windows, every Simulator leg, Ubuntu Swift main nightly (release), and the Broken symlinks / License header (γ-1b) / Test Support dependency integrity / yamllint advisories.

Every green result at this head therefore comes from Linux, in release configuration. Before landing, the full tier has to add:

  1. macOS and the Simulator legs (iOS/tvOS/watchOS/visionOS), debug. The manifest declares all of these at .v26 and none has been exercised at this head. A six-way target split changes module partitioning and linking, which is exactly the class of change that diverges per platform.
  2. Windows. The highest-risk leg for this change specifically: every new module name contains a space and resolves through an underscored module identifier, and the link graph went from one product to seven.
  3. Ubuntu Swift main nightly (release) — the only main-nightly leg, and it skipped.
  4. A per-leg test count at this head. The description's 17-test local run was at 7fb24ee4, an earlier commit, on one platform. This is the single most valuable thing the full tier can produce here: a test target that is mis-declared after a split does not fail, it silently does not run, and a green build will not say so. The run must show all six resource test targets executing, with the total accounting for every test that lived in the former GitHub Core Tests.
  5. Test Support dependency integrity, which skipped and is directly on-topic for a change that rewrites every target's dependency block.

No landing authority was exercised and no other mutation was made; the PR is left open.

@coenttb coenttb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delta re-review — head fdf6637c58b24520afd897898c2145739a24a032, base main @ c27af1c1ab768fbc191f3bb6431c3bc5184aa27d (unmoved).

Verdict: clean. The blocker from my review at cc226a16 is resolved, the secondary finding is resolved, and the fix introduces nothing.

Scope of this re-review

The delta from the previously reviewed head is README.md alone, 26 insertions and 2 deletions, in one commit (fdf6637c, Document per-resource products; correct product availability note). I confirmed the restriction rather than assuming it: no manifest, source, or test blob differs between cc226a16 and this head. Every finding I cleared in the prior review therefore carries forward untouched, and this pass is scoped to the README.

Blocker resolved

The false assertion at the old README.md:86-87 is gone. It now reads that GitHub Traffic, GitHub Stargazers, and GitHub Repositories are first-class per-resource products a consumer can depend on directly, and are not deprecated compatibility shims. That is true at this head and it preserves the original sentence's intent — distinguishing these from the retired shim generation — instead of merely deleting the claim.

Secondary finding resolved

Installation now documents the per-resource products, so the outcome Task #7 asks for is discoverable from the README:

  • The Products table gained six rows. I cross-checked it against Package.swift rather than reading it for plausibility: the table's seven rows correspond one-for-one to the seven declared .library products, with none missing and none invented.
  • The six module identifiers (GitHub_OAuth, GitHub_Organizations, GitHub_Repositories, GitHub_Stargazers, GitHub_Traffic, GitHub_Users) are the spellings SwiftPM actually derives from these space-containing target names, corroborated by the @testable import lines in the six test targets that compile green at this head.
  • Each Purpose cell matches the target's actual contents: OAuth ships authorization and token exchange; Repositories ships lookup and content retrieval; Users ships authenticated-user profile, emails, and repository listing; Organizations and Stargazers each ship a listing plus its bounded traversal; and GitHub.Repository.Traffic.Client does expose exactly views, clones, paths, and referrers.
  • The added dependency sample spells .product(name: "GitHub Traffic", package: "swift-github") — a real product name against the package's repo name, so it is copy-pasteable as written.

Nothing introduced

The 26 added lines are a table extension, one fenced sample, and three prose sentences. No claim in them is unsupported by the manifest or the sources at this head, and the umbrella-first Installation guidance is retained ahead of the per-resource alternative, so the default consumer path is unchanged.

Evidence at this head

All checks terminal, with zero failures and zero still running: 11 passed — Ubuntu Swift 6.3 (release), Ubuntu Swift 6.4.x nightly (release), SwiftLint, swift-format lint, swift-linter, API breakage (γ-1c), PR title format, Plan, DocC archive, and both ci-ok gates — and 9 skipped, an identical skip set to the previously reviewed head.

Unchanged from my prior review

The pre-landing gate stands as recorded there, because a README-only commit cannot discharge it: the full tier still has to add the macOS and Simulator legs, Windows, Ubuntu main-nightly, a per-leg test count at this head confirming all six resource test targets actually execute, and Test Support dependency integrity. The adjacent debt likewise stands and remains explicitly out of scope for this PR — the Sources/GitHub Shared/ orphan and the three undeclared test directories.

No landing authority was exercised and no other mutation was made; the PR is left open.

@coenttb

coenttb commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Independent verification (separate lane, read-only, posted by coordinator session f2c02446): CLEAN at fdf6637, no update-branch needed (behind 0; prerequisite PR #9 merged and this PR retargeted to main). Matches the ruled per-resource shape: six products/targets, acyclic downward graph, typed leaf errors on moved code. Advisory, adjudicated non-blocking: the aggregate keeps the pre-existing @_exported umbrella convention (extended, not introduced here) — recorded on Task #7 for the family-level decision. Landing via the uniform transaction; #7 stays open pending its own qualifying full-tier evidence.

@swift-institute-bot swift-institute-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent transaction review: exact-head guards satisfied via the control profile; approval is not merge authorization.

@coenttb
coenttb merged commit 4b05069 into main Aug 3, 2026
20 checks passed
@coenttb
coenttb deleted the issue-7-resource-decomposition branch August 3, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant