chore: consolidate project metadata, drop unused dependencies and vendored sample assets #142

Merged
Grandiras merged 1 commit from issue-110-hygiene into main 2026-09-24 17:52:56 +00:00
Owner

Closes #110.

Cleanup the v0.6.0 sweep missed. Nothing here should change behaviour. The only effect on consumers is two dependencies the core package never used.

Project files

  • Shared settings are set once. The root Directory.Build.props now holds TargetFramework, Nullable, ImplicitUsings and all package metadata. The unused root PackageTags is gone, because every packable project overrides it. Three new layers each import the root file:
    • src/Directory.Build.props sets GenerateDocumentationFile and makes CS1591 an error.
    • tests/Directory.Build.props sets IsPackable=false, the xUnit1051 suppression and the global using Xunit;.
    • samples/Directory.Build.props sets IsPackable=false.
  • Settings that equalled the SDK default are removed: LangVersion latest, TreatWarningsAsErrors false, and RootNamespace/AssemblyName/PackageId wherever they equalled the project name. The project files now keep only Description, PackageTags and their references.
  • Two unused core dependencies are gone. ATProtoNet.csproj drops Microsoft.Extensions.Http and Microsoft.Extensions.Options. IHttpClientFactory only appeared in <c> doc text. The core dependency closure goes from 14 packages to 3: Logging.Abstractions, System.Formats.Cbor, and DependencyInjection.Abstractions under the first. ATProtoNet.Server's references are untouched.
  • Redundant transitive ProjectReferences to core are removed from ManagedPdsSample, ServerIntegrationSample and ATProtoNet.IntegrationTests. ServerIntegrationSample keeps its explicit ATProtoNet.Server reference even though it arrives through Blazor today, because the sample calls Server APIs directly and the package restructure may cut Blazor→Server.
  • atproto-lexgen --version and the --help banner print AssemblyInformationalVersionAttribute instead of a hard-coded "1.0.0".
  • .gitignore: the duplicate .idea/ line is removed and nupkg/ is added.

Dead code

  • Unnecessary usings (IDE0005) are removed in 23 files across src/, tests/ and tools/. They were applied with dotnet format style --diagnostics IDE0005, not by hand. The build diagnostic reports only the first line of each contiguous unnecessary block, so the report's count of 42 undercounts. FeedClient.cs and FirehoseClient.cs each had 5.
  • Unused internals are removed:
    • XrpcClient.RefreshToken.
    • StandardSiteClient._xrpc, together with its now-unused internal constructor parameter. The call sites in AtProtoClient and StandardSiteClientTests are updated.
    • The currentNonce parameter of OAuthClient.SendWithDpopRetryAsync.
    • In the Lexicon generator: the namespacePrefix parameter of EmitPlan.PlanUnions, the hint parameter of EmitPlan.WalkObject, and the defName parameter of LexiconEmitter.BuildSchemaFromType.

Samples

  • Vendored Bootstrap trees are removed. Across both samples that is 87 of 88 tracked files and 17.1 MB. Only wwwroot/lib/bootstrap/dist/css/bootstrap.min.css stays, at the same path, so App.razor is unchanged and the sample still works offline. Its license banner is in the file header.

  • samples/BlazorOAuthSample is folded into samples/ServerIntegrationSample. Before the fold, a diff showed three things only BlazorOAuthSample had, and all three moved over:

    • app.UseStatusCodePagesWithReExecute("/not-found", …), so unknown URLs render the NotFound page again. It had been unreachable in ServerIntegrationSample.
    • appsettings.json and appsettings.Development.json, moved with git mv, so the log levels match.
    • The "Auth Method" row on the home page, plus the explanatory comments on the OAuth registration and endpoints.

    The solution file and the links in docs/blazor.md, docs/oauth.md and docs/architecture.md are updated.

  • Template leftovers are removed:

    • ReconnectModal.razor, .css and .js (251 lines). Blazor shows its built-in reconnect UI instead.
    • Unused app.css rules: form validation, blazor-error-boundary, and the Identity-template darker-border-checkbox and form-floating rules.
    • The unused .bi-list-nested-nav-menu icon rule.
    • Five unused _Imports.razor directives.

    MainLayout.razor.css and NavMenu.razor.css stay, because the sidebar layout uses them.

  • Docs and CLAUDE.md: the "Shared build config" section of docs/architecture.md and CLAUDE.md describe the new Directory.Build.props layering.

Breaking changes

  • ATProtoNet no longer brings Microsoft.Extensions.Http or Microsoft.Extensions.Options in transitively. ASP.NET Core apps are unaffected, because the shared framework carries both.
    Migration: an app outside ASP.NET Core that used IHttpClientFactory, IOptions<T> or LoggerFactory only through this transitive dependency adds Microsoft.Extensions.Http, Microsoft.Extensions.Options or Microsoft.Extensions.Logging itself.

There are no public API changes. Every removed member or parameter is internal or private.

Verification

  • Build: dotnet build of the full solution with --no-incremental finishes with 0 warnings and 0 errors.
  • Unit tests: 1,710 passed, 0 failed, 5 skipped, out of 1,715. Two of them are new: LexiconGeneratorProgramTests.Main_VersionFlag_PrintsPackageVersion for --version and -v. It checks that the output starts with the assembly's Major.Minor.Build, so it fails against the old 1.0.0.
  • Pack comparison: dotnet pack -c Release ran before and after, and every .nupkg and .snupkg was compared by nuspec and file list. The only difference is the two removed <dependency> entries in ATProtoNet.0.6.0.nupkg and .snupkg. Authors, license, URLs, readme, tags, descriptions, the tool package type, and the file lists (DLLs, XML docs, README) are identical for all five packages.
  • IDE diagnostics: a rebuild with EnforceCodeStyleInBuild and IDE0005/0052/0060 promoted to warnings leaves IDE0005 hits only in the parallel-owned files listed below.
  • ServerIntegrationSample was run on 127.0.0.1:5010:
    • /, /login and /not-found return 200.
    • /does-not-exist returns 404 with the NotFound page.
    • bootstrap.min.css, app.css and the scoped CSS are served.
    • /profile without a session redirects 302 to /login?ReturnUrl=%2Fprofile.
    • /atproto/login without a handle returns 400.
    • /atproto/login?handle=bsky.app runs the full PAR flow and redirects 302 to https://bsky.social/oauth/authorize with the loopback client_id and redirect_uri=http://127.0.0.1:5010/atproto/callback. This is the same path BlazorOAuthSample took on port 5000.
    • /atproto/callback?error=… redirects to /login?error=….
    • The login page renders with the sidebar layout and no console errors.

Deliberately deferred

  • Linking SpacesSample from docs/spaces.md is left to the Spaces PR.
  • .github/workflows/ci.yml and nuget.config are kept on the owner's instructions.
  • Files owned by parallel PRs are left untouched:
    • src/ATProtoNet.Aspire.Hosting/ATProtoNet.Aspire.Hosting.csproj and samples/SpacesSample/SpacesSample.csproj still set properties that now come from the props files (TFM, nullable, usings, doc file, LangVersion, and so on). That is harmless but redundant, and a one-line trim for whichever PR touches them.
    • Their IDE0005 hits stay, in Crypto/, Repo/, Serialization/, *Models.cs, Server/Spaces/, SpacesSample, and the matching tests.
  • Not in the issue's list, so not touched:
    • AtProtoAuthenticationHandler._client (IDE0052), whose file goes away in the package restructure.
    • The public AtProtoOAuthService cancellationToken parameter.
    • MerkleSearchTree depth, owned by the Repo PR.
  • A pre-existing gap in the sample, not fixed here: the Profile and Timeline nav links use bi-person-fill-nav-menu and bi-list-check-nav-menu, which no CSS defines, so those two entries render without icons.
  • docs/architecture.md's samples tree still omits JetstreamReplaySample and SpacesSample. That is left to the docs overhaul.

🤖 Generated with Claude Code

Closes #110. Cleanup the v0.6.0 sweep missed. Nothing here should change behaviour. The only effect on consumers is two dependencies the core package never used. ## Project files - **Shared settings are set once.** The root `Directory.Build.props` now holds `TargetFramework`, `Nullable`, `ImplicitUsings` and all package metadata. The unused root `PackageTags` is gone, because every packable project overrides it. Three new layers each import the root file: - `src/Directory.Build.props` sets `GenerateDocumentationFile` and makes CS1591 an error. - `tests/Directory.Build.props` sets `IsPackable=false`, the `xUnit1051` suppression and the global `using Xunit;`. - `samples/Directory.Build.props` sets `IsPackable=false`. - **Settings that equalled the SDK default are removed:** `LangVersion latest`, `TreatWarningsAsErrors false`, and `RootNamespace`/`AssemblyName`/`PackageId` wherever they equalled the project name. The project files now keep only `Description`, `PackageTags` and their references. - **Two unused core dependencies are gone.** `ATProtoNet.csproj` drops `Microsoft.Extensions.Http` and `Microsoft.Extensions.Options`. `IHttpClientFactory` only appeared in `<c>` doc text. The core dependency closure goes from 14 packages to 3: `Logging.Abstractions`, `System.Formats.Cbor`, and `DependencyInjection.Abstractions` under the first. `ATProtoNet.Server`'s references are untouched. - **Redundant transitive `ProjectReference`s to core are removed** from `ManagedPdsSample`, `ServerIntegrationSample` and `ATProtoNet.IntegrationTests`. `ServerIntegrationSample` keeps its explicit `ATProtoNet.Server` reference even though it arrives through Blazor today, because the sample calls Server APIs directly and the package restructure may cut Blazor→Server. - **`atproto-lexgen --version`** and the `--help` banner print `AssemblyInformationalVersionAttribute` instead of a hard-coded `"1.0.0"`. - **`.gitignore`:** the duplicate `.idea/` line is removed and `nupkg/` is added. ## Dead code - **Unnecessary `using`s (IDE0005)** are removed in 23 files across `src/`, `tests/` and `tools/`. They were applied with `dotnet format style --diagnostics IDE0005`, not by hand. The build diagnostic reports only the first line of each contiguous unnecessary block, so the report's count of 42 undercounts. `FeedClient.cs` and `FirehoseClient.cs` each had 5. - **Unused internals are removed:** - `XrpcClient.RefreshToken`. - `StandardSiteClient._xrpc`, together with its now-unused internal constructor parameter. The call sites in `AtProtoClient` and `StandardSiteClientTests` are updated. - The `currentNonce` parameter of `OAuthClient.SendWithDpopRetryAsync`. - In the Lexicon generator: the `namespacePrefix` parameter of `EmitPlan.PlanUnions`, the `hint` parameter of `EmitPlan.WalkObject`, and the `defName` parameter of `LexiconEmitter.BuildSchemaFromType`. ## Samples - **Vendored Bootstrap trees are removed.** Across both samples that is 87 of 88 tracked files and 17.1 MB. Only `wwwroot/lib/bootstrap/dist/css/bootstrap.min.css` stays, at the same path, so `App.razor` is unchanged and the sample still works offline. Its license banner is in the file header. - **`samples/BlazorOAuthSample` is folded into `samples/ServerIntegrationSample`.** Before the fold, a diff showed three things only BlazorOAuthSample had, and all three moved over: - `app.UseStatusCodePagesWithReExecute("/not-found", …)`, so unknown URLs render the NotFound page again. It had been unreachable in ServerIntegrationSample. - `appsettings.json` and `appsettings.Development.json`, moved with `git mv`, so the log levels match. - The "Auth Method" row on the home page, plus the explanatory comments on the OAuth registration and endpoints. The solution file and the links in `docs/blazor.md`, `docs/oauth.md` and `docs/architecture.md` are updated. - **Template leftovers are removed:** - `ReconnectModal.razor`, `.css` and `.js` (251 lines). Blazor shows its built-in reconnect UI instead. - Unused `app.css` rules: form validation, `blazor-error-boundary`, and the Identity-template `darker-border-checkbox` and `form-floating` rules. - The unused `.bi-list-nested-nav-menu` icon rule. - Five unused `_Imports.razor` directives. `MainLayout.razor.css` and `NavMenu.razor.css` stay, because the sidebar layout uses them. - **Docs and `CLAUDE.md`:** the "Shared build config" section of `docs/architecture.md` and `CLAUDE.md` describe the new `Directory.Build.props` layering. ## Breaking changes - **`ATProtoNet` no longer brings `Microsoft.Extensions.Http` or `Microsoft.Extensions.Options` in transitively.** ASP.NET Core apps are unaffected, because the shared framework carries both. Migration: an app outside ASP.NET Core that used `IHttpClientFactory`, `IOptions<T>` or `LoggerFactory` only through this transitive dependency adds `Microsoft.Extensions.Http`, `Microsoft.Extensions.Options` or `Microsoft.Extensions.Logging` itself. There are no public API changes. Every removed member or parameter is internal or private. ## Verification - **Build:** `dotnet build` of the full solution with `--no-incremental` finishes with 0 warnings and 0 errors. - **Unit tests:** 1,710 passed, 0 failed, 5 skipped, out of 1,715. Two of them are new: `LexiconGeneratorProgramTests.Main_VersionFlag_PrintsPackageVersion` for `--version` and `-v`. It checks that the output starts with the assembly's `Major.Minor.Build`, so it fails against the old `1.0.0`. - **Pack comparison:** `dotnet pack -c Release` ran before and after, and every `.nupkg` and `.snupkg` was compared by nuspec and file list. The only difference is the two removed `<dependency>` entries in `ATProtoNet.0.6.0.nupkg` and `.snupkg`. Authors, license, URLs, readme, tags, descriptions, the tool package type, and the file lists (DLLs, XML docs, README) are identical for all five packages. - **IDE diagnostics:** a rebuild with `EnforceCodeStyleInBuild` and IDE0005/0052/0060 promoted to warnings leaves IDE0005 hits only in the parallel-owned files listed below. - **ServerIntegrationSample was run on `127.0.0.1:5010`:** - `/`, `/login` and `/not-found` return 200. - `/does-not-exist` returns 404 with the NotFound page. - `bootstrap.min.css`, `app.css` and the scoped CSS are served. - `/profile` without a session redirects 302 to `/login?ReturnUrl=%2Fprofile`. - `/atproto/login` without a handle returns 400. - `/atproto/login?handle=bsky.app` runs the full PAR flow and redirects 302 to `https://bsky.social/oauth/authorize` with the loopback `client_id` and `redirect_uri=http://127.0.0.1:5010/atproto/callback`. This is the same path BlazorOAuthSample took on port 5000. - `/atproto/callback?error=…` redirects to `/login?error=…`. - The login page renders with the sidebar layout and no console errors. ## Deliberately deferred - **Linking `SpacesSample` from `docs/spaces.md`** is left to the Spaces PR. - **`.github/workflows/ci.yml` and `nuget.config`** are kept on the owner's instructions. - **Files owned by parallel PRs are left untouched:** - `src/ATProtoNet.Aspire.Hosting/ATProtoNet.Aspire.Hosting.csproj` and `samples/SpacesSample/SpacesSample.csproj` still set properties that now come from the props files (TFM, nullable, usings, doc file, `LangVersion`, and so on). That is harmless but redundant, and a one-line trim for whichever PR touches them. - Their IDE0005 hits stay, in `Crypto/`, `Repo/`, `Serialization/`, `*Models.cs`, `Server/Spaces/`, `SpacesSample`, and the matching tests. - **Not in the issue's list, so not touched:** - `AtProtoAuthenticationHandler._client` (IDE0052), whose file goes away in the package restructure. - The public `AtProtoOAuthService` `cancellationToken` parameter. - `MerkleSearchTree` `depth`, owned by the Repo PR. - **A pre-existing gap in the sample, not fixed here:** the Profile and Timeline nav links use `bi-person-fill-nav-menu` and `bi-list-check-nav-menu`, which no CSS defines, so those two entries render without icons. - **`docs/architecture.md`'s samples tree** still omits `JetstreamReplaySample` and `SpacesSample`. That is left to the docs overhaul. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
chore: consolidate project metadata, drop unused dependencies and vendored sample assets (closes #110)
All checks were successful
CI / pds-integration (pull_request) Successful in 25s
CI / build-and-test (pull_request) Successful in 57s
Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s
d4e196d60e
Hygiene the v0.6.0 sweep missed, with no intended behaviour change.

Project files: the target framework, nullable, implicit usings and the
package metadata move into Directory.Build.props, with src/, tests/ and
samples/ layers for the documentation file (CS1591 stays an error) and
IsPackable. Settings that restated SDK defaults are gone, as are the
transitive ProjectReferences in the samples and integration tests. The
core package drops Microsoft.Extensions.Http and .Options, which it never
used, taking its dependency closure from 14 packages to 3. dotnet pack
output is otherwise identical: every nuspec and file list matches.

Dead code: unnecessary usings outside the areas other PRs are rewriting,
XrpcClient.RefreshToken, StandardSiteClient's unread XrpcClient, and
unused private parameters in OAuthClient and the Lexicon generator.
atproto-lexgen --version now reports the informational version instead
of a hard-coded 1.0.0.

Samples: BlazorOAuthSample is folded into ServerIntegrationSample, which
was already a superset; the status-code re-execution, the auth-method row
and the logging config carry over. Only bootstrap.min.css is kept from the
17 MB vendored Bootstrap tree, and the template's reconnect dialog, unused
CSS and unused imports go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript. <!-- opencode-run:review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-142/run-2026-09-24T12-47-12-387Z.md) — updating every ~5 minutes while the review runs.
opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

A clean, well-verified chore. I spot-checked the substantive claims and they all hold:

  • Props layering is correct. The src/, tests/ and samples/ Directory.Build.props files chain up to the root via $([MSBuild]::GetPathOfFileAbove(...)), so every project gets exactly one copy of the shared settings. $(WarningsAsErrors);CS1591 is valid with TreatWarningsAsErrors left unset (the list is honored independently). The redundant settings that remain in ATProtoNet.Aspire.Hosting.csproj and SpacesSample.csproj are the two disclosed deferred files, and they agree with the inherited values, so they're inert.
  • The dependency removal is real. IHttpClientFactory survives only inside <c> doc text; ILoggerFactory usage is covered by the retained Logging.Abstractions. I verified against the Microsoft.Extensions.Http 10.0.10 nuspec that it does transitively bring Microsoft.Extensions.Logging (the implementation) on net10.0, so the CHANGELOG migration note mentioning LoggerFactory is accurate.
  • The internal removals are consistent. No references remain to XrpcClient.RefreshToken, StandardSiteClient._xrpc/its ctor parameter, or currentNonce on SendWithDpopRetryAsync (the DPoP nonce now flows through an updateNonce callback at both call sites). The generator's trimmed signatures match their call sites.
  • The sample fold is complete. The merged ServerIntegrationSample has the status-code pages handler, the git mv-ed appsettings*.json, the "Auth Method" row (and the pds_url/auth_method claim names it reads are what AtProtoOAuthService sets), no ReconnectModal, and no dangling references to the removed template CSS or imports. Only bootstrap.min.css is vendored, with its license banner. The ATProtoNet.Server reference is retained as explained. slnx and the doc links are updated; the only remaining BlazorOAuthSample mention is a historical CHANGELOG entry, which is right to leave alone.
  • atproto-lexgen --version now reads AssemblyInformationalVersionAttribute, and the new test compiles (the Tests project references the tool project); StartsWith correctly tolerates any informational-version suffix.
  • .gitignore: .idea/ is still ignored (the duplicate line was the one removed) and nupkg/ is added.

Minor notes, none blocking: the Console.SetOut swap in LexiconGeneratorProgramTests is inherently racy with parallel tests, but the filtering Assert.Single(...) mitigates the visible symptom and the risk is contained to test output. Two deferred one-line trims (Aspire.Hosting, SpacesSample) remain as disclosed for their owning PRs.


⏱ 6m 6s · 29 steps · $0.1455 · tokens: 519.7k in / 11.8k out (+2034.0k cache-read) · Go: 5h 17% · wk 93% · mo 79%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. A clean, well-verified chore. I spot-checked the substantive claims and they all hold: - **Props layering is correct.** The `src/`, `tests/` and `samples/` `Directory.Build.props` files chain up to the root via `$([MSBuild]::GetPathOfFileAbove(...))`, so every project gets exactly one copy of the shared settings. `$(WarningsAsErrors);CS1591` is valid with `TreatWarningsAsErrors` left unset (the list is honored independently). The redundant settings that remain in `ATProtoNet.Aspire.Hosting.csproj` and `SpacesSample.csproj` are the two disclosed deferred files, and they agree with the inherited values, so they're inert. - **The dependency removal is real.** `IHttpClientFactory` survives only inside `<c>` doc text; `ILoggerFactory` usage is covered by the retained `Logging.Abstractions`. I verified against the `Microsoft.Extensions.Http` 10.0.10 nuspec that it does transitively bring `Microsoft.Extensions.Logging` (the implementation) on net10.0, so the CHANGELOG migration note mentioning `LoggerFactory` is accurate. - **The internal removals are consistent.** No references remain to `XrpcClient.RefreshToken`, `StandardSiteClient._xrpc`/its ctor parameter, or `currentNonce` on `SendWithDpopRetryAsync` (the DPoP nonce now flows through an `updateNonce` callback at both call sites). The generator's trimmed signatures match their call sites. - **The sample fold is complete.** The merged `ServerIntegrationSample` has the status-code pages handler, the `git mv`-ed `appsettings*.json`, the "Auth Method" row (and the `pds_url`/`auth_method` claim names it reads are what `AtProtoOAuthService` sets), no `ReconnectModal`, and no dangling references to the removed template CSS or imports. Only `bootstrap.min.css` is vendored, with its license banner. The `ATProtoNet.Server` reference is retained as explained. slnx and the doc links are updated; the only remaining `BlazorOAuthSample` mention is a historical CHANGELOG entry, which is right to leave alone. - **`atproto-lexgen --version`** now reads `AssemblyInformationalVersionAttribute`, and the new test compiles (the Tests project references the tool project); `StartsWith` correctly tolerates any informational-version suffix. - **.gitignore**: `.idea/` is still ignored (the duplicate line was the one removed) and `nupkg/` is added. Minor notes, none blocking: the `Console.SetOut` swap in `LexiconGeneratorProgramTests` is inherently racy with parallel tests, but the filtering `Assert.Single(...)` mitigates the visible symptom and the risk is contained to test output. Two deferred one-line trims (Aspire.Hosting, SpacesSample) remain as disclosed for their owning PRs. --- ⏱ 6m 6s · 29 steps · $0.1455 · tokens: 519.7k in / 11.8k out (+2034.0k cache-read) · Go: 5h 17% · wk 93% · mo 79% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-142/run-2026-09-24T12-47-12-387Z.md) <!-- opencode-reviewed-head:d4e196d60e733afe54d8d1344ea442743ea6b95e --> <!-- opencode-verdict:approve -->
Grandiras deleted branch issue-110-hygiene 2026-09-24 17:52:56 +00:00
Sign in to join this conversation.
No description provided.