Repository navigation
chore(gitsync): drop the transitional Octokit reference, overload and pin - #7265
Merged
Merged
Conversation
… pin No core code has used Octokit since the GitHub REST client became a plain HttpClient. The bare PackageReference, the PackageVersion and the internal IsMissingOrEmptyRepo(Octokit.ApiException) overload stayed for one cycle so the image kept carrying Octokit.dll. Both removal conditions in Doc/Architecture/GitHubSync now hold: 1. MeshWeaver.Plugins#4206 merged; nothing in Plugins names an Octokit type. 2. The Observability builds published after it are what instances run: the package-update holds on pearl and build were resolved when those instances rolled past the floor, and an image without Octokit reaches an instance only through such a roll. No satellite names an Octokit type (Memex carries only its own unused PackageVersion). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Safe removal of the runtime DLL depends on deployed Observability bundles adopting the replacement, which remains independently unverified.
No open issues 🎉
What changed in this PR
Completes the GitSync migration to the HTTP-based GitHub REST client by removing the temporary Octokit compatibility dependency.
Changes:
- Removes the Octokit package reference and central version pin.
- Deletes the transitional internal overload accepting
Octokit.ApiException. - Updates the architecture documentation with the reported downstream migration and deployment evidence.
| File | Description |
|---|---|
src/MeshWeaver.GitSync/OctokitGitHubRepoClient.cs |
Removes the Octokit exception overload. |
src/MeshWeaver.GitSync/MeshWeaver.GitSync.csproj |
Removes the transitional package reference. |
src/MeshWeaver.Documentation/Data/Architecture/GitHubSync.md |
Documents removal and its prerequisites. |
Directory.Packages.props |
Removes the Octokit version pin. |
Review effort: Balanced
Contributor
Test Results 25 files ±0 25 suites ±0 55m 42s ⏱️ +17s Results for commit 8273695. ± Comparison against base commit 09ed616. This pull request removes 59 and adds 22 tests. Note that renamed tests count towards both. |
rbuergi
added a commit
that referenced
this pull request
Oct 26, 2026
Resolves Directory.Packages.props: main dropped the transitional Octokit pin (#7265) and this branch dropped NSubstitute; both lines go. PackageFamiliesMoveTogetherGuard.TheScannerSeesEveryFamilyItPolices failed on this branch (shard 7): it required more than 100 pins and at least two Aspire.* entries, and core now pins 97 and no Aspire package. The Aspire prefix leaves the core guard (its pins live in Plugins since Plugins#4297) and the count becomes a floor of 50 that catches a misread regex without moving each time a pin leaves for the repo that uses it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rbuergi
added a commit
that referenced
this pull request
Oct 26, 2026
* chore(packages): core pins only what core restores (delete half) Drops 48 PackageVersion entries that no core project restores at any depth, measured from every project.assets.json of a full solution restore, plus MeshWeaver.Graph's Humanizer.Core reference, which nothing in that project calls. MeshWeaver.Plugins now owns those pins as fill-in entries. Kept on purpose: 12 transitive pins core's own graph carries (Azure.Core, System.ClientModel, …), and the families core still partly pins (Orleans clustering/persistence, the ModelContextProtocol trio, Microsoft.Agents.AI, Microsoft.Extensions.AI). Radzen.Blazor and the other shelf pins leave with the module-library shelf; Octokit leaves on its own two conditions. Doc: CarvingProjectsOutOfCore, "Package versions follow the projects". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Merge origin/main; the family guard follows core's remaining pins Resolves Directory.Packages.props: main dropped the transitional Octokit pin (#7265) and this branch dropped NSubstitute; both lines go. PackageFamiliesMoveTogetherGuard.TheScannerSeesEveryFamilyItPolices failed on this branch (shard 7): it required more than 100 pins and at least two Aspire.* entries, and core now pins 97 and no Aspire package. The Aspire prefix leaves the core guard (its pins live in Plugins since Plugins#4297) and the count becomes a floor of 50 that catches a misread regex without moving each time a pin leaves for the repo that uses it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The platform no longer references Octokit.
This removes the transitional parts that kept
Octokit.dllin the image for one cycle after the GitHub REST client was ported to a plainHttpClient:<PackageReference Include="Octokit" />inMeshWeaver.GitSync;OctokitPackageVersion;OctokitGitHubRepoClient.IsMissingOrEmptyRepo(Octokit.ApiException).Both removal conditions from
Doc/Architecture/GitHubSynchold:main, everyOctokithit is a comment or core'sOctokitGitHubRepoClientclass name, which stays.pearl(1.8.95 held below 1.8.104) andbuild(1.8.99 held below 1.8.100).Hosting/Triage/package-update-held/pearl-observabilityand.../build-observability).Satellites. I searched every
.cs,.csproj,.propsand.jsonfile in shallow clones of the default branches of Education, Reinsurance, SocialMedia, Manufacturing, Crm, FundReporting, Memex and MeshWeaver.Deploy. The only hit is Memex's ownPackageVersion Include="Octokit", which nothing there references.Verified locally, Release with
-warnaserror:MeshWeaver.GitSync,MeshWeaver.PluginCatalog,MeshWeaver.ComboAssembler,MeshWeaver.ComboVerifierandMeshWeaver.Hosting.Testbuild with 0 warnings and 0 errors.GitHubRestTransportTest: 53 of 53 pass.Doc.
GitHubSync: "Removing the transitional Octokit reference" is now "The transitional Octokit reference, removed", with the evidence above.Pairs-with: Systemorph/MeshWeaver.Plugins#4206 — merged; it ported Plugins off the Octokit types and off the removed internal overload.
Satellite-pins: Octokit — searched every satellite's default branch as above. No versionless consumer: Plugins references no Octokit package, and Memex pins its own unused version.
🤖 Generated with Claude Code