Repository navigation
Retire the remaining IO.Ably-named build artifacts - #1343
Merged
Merged
Conversation
The strong-name key and the two StyleCop rulesets were the last live build artifacts still named IO.Ably. Rename them to match everything else: IO.Ably.snk -> Ably.PubSub.snk, src/IO.Ably.ruleset -> src/Ably.PubSub.ruleset, src/IO.Ably.Tests.ruleset -> src/Ably.PubSub.Tests.ruleset, with their internal RuleSet Name and Description metadata updated to match. The key file's bytes are unchanged, so the assembly signing identity and every InternalsVisibleTo PublicKey grant are unaffected - only the file name moves. The NETFramework core project carried a byte-identical copy of the key (verified by checksum); it is consolidated away and the project now signs with the repo-root key like every other project. The whole reference web moves in step: AssemblyOriginatorKeyFile in all five signing csproj files, CodeAnalysisRuleSet in all nine product, test and example csproj files, the solution-items entries in all four solutions, the ILRepack keyfile path in the cake helper, generate_key.bat and the build guidance in CLAUDE.md. The AssemblyInfo comments that name the key file are updated too: an earlier review fix restored them to the then-true IO.Ably.snk, and this rename moves the file to what those comments always meant, so they now read Ably.PubSub.snk and stay accurate. IO.Ably survives only where it denotes the 1.x past (CHANGELOG history, the migration guide's old-name column, IoC's history note) or the external IO.Ably.DeltaCodec submodule.
src/Ably.PubSub/CustomSerialisers/GeneratedSerializers/IO/Ably/... held three MsgPack serializer files produced by a long-ago generator run. No project compiles them - they appear in no projitems or csproj compile item - and their directory path still spelled out the old IO/Ably namespace segments. The live copies of the two overlapping serializers ship from Ably.PubSub.Shared.MsgPack; the third (IO_Ably_CapabilityResourceSerializer) is referenced nowhere at all. The only mention of the directory is the OutputDirectory of the MsgPack serializer generator scaffold in MsgPackMessageSerializerTests - a [Fact(Skip="true")] test filtered to a single type. That scaffold is dead scenery of the same scenery: its output path now points at a directory that no longer exists, which is harmless for a permanently skipped test and becomes moot when the MSGPACK compile path's remove-or-resurrect decision lands, so the path is deliberately left untouched.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (24)
💤 Files with no reviewable changes (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughBuild and signing references now use Ably.PubSub ruleset and key names. Three generated MessagePack serializers for CapabilityResource, Capability, and TokenRequest were removed. ChangesBuild identity and serializer cleanup
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to The renamed build references preserve the existing signing identity, and the removed serializers are not used by the current configured MessagePack path. No concrete change-introduced issue remains; the PR appears mergeable subject to normal checks. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 5 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the keys at dawn, Comment |
Base automatically changed from
integration/namespace-ably-pubsub
to
integration/v2
September 29, 2026 13:12
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Renames and deletions are validated with no unresolved blocking issues.
Review effort: Lite
Findings: None
What changed in this PR
Retires legacy IO.Ably build-artifact names while preserving signing identity and removing orphaned generated serializers.
Changes:
- Renamed signing key and rulesets, updating project, solution, script, and documentation references.
- Consolidated .NET Framework signing onto the shared key.
- Deleted three unreferenced generated serializer files.
| File | Change |
|---|---|
src/Ably.PubSub/CustomSerialisers/GeneratedSerializers/IO/Ably/CustomSerialisers/IO_Ably_TokenRequestSerializer.cs |
Removed orphaned serializer |
src/Ably.PubSub/CustomSerialisers/GeneratedSerializers/IO/Ably/CustomSerialisers/IO_Ably_CapabilitySerializer.cs |
Removed orphaned serializer |
src/Ably.PubSub/CustomSerialisers/GeneratedSerializers/IO/Ably/CustomSerialisers/IO_Ably_CapabilityResourceSerializer.cs |
Removed orphaned serializer |
src/Ably.PubSub.Tests.ruleset |
Renamed test ruleset |
src/Ably.PubSub.Tests.NETFramework/Ably.PubSub.Tests.NETFramework.csproj |
Updated ruleset reference |
src/Ably.PubSub.Tests.DotNET/Ably.PubSub.Tests.DotNET.csproj |
Updated ruleset reference |
src/Ably.PubSub.sln |
Updated ruleset entry |
src/Ably.PubSub.Server/Ably.PubSub.Server.csproj |
Updated build references |
src/Ably.PubSub.Server.NETFramework/Ably.PubSub.Server.NETFramework.csproj |
Updated build and signing references |
src/Ably.PubSub.ruleset |
Renamed product ruleset |
src/Ably.PubSub.Package.sln |
Updated ruleset entry |
src/Ably.PubSub.NetStandard.sln |
Updated ruleset entry |
src/Ably.PubSub.NetFramework.sln |
Updated ruleset entry |
src/Ably.PubSub.Device/Ably.PubSub.Device.csproj |
Updated build references |
src/Ably.PubSub.Core/Properties/AssemblyInfo.cs |
Updated key comment |
src/Ably.PubSub.Core/Ably.PubSub.Core.csproj |
Updated ruleset and signing references |
src/Ably.PubSub.Core.NETFramework/Properties/AssemblyInfo.cs |
Updated key comment |
src/Ably.PubSub.Core.NETFramework/Ably.PubSub.Core.NETFramework.csproj |
Consolidated signing-key reference |
generate_key.bat |
Generates renamed key |
examples/NotificationsPublisher/NotificationsPublisher.csproj |
Updated ruleset reference |
CLAUDE.md |
Updated ruleset documentation |
cake-build/helpers/tools.cake |
Updated ILRepack key path |
Ably.PubSub.snk |
Renamed shared signing key |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ttypic
approved these changes
Sep 29, 2026
6 tasks
This branch was successfully deployed
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.
What this PR does
Retires the last live
IO.Ably-named build artifacts, so the old name survives only where it denotes the 1.x past or an external dependency. Two commits: the artifact renames, and the deletion of three orphaned generated files. Follows #1342, which is merged intointegration/v2(54eff1bd).The renames
IO.Ably.snk(repo root)Ably.PubSub.snkAssemblyOriginatorKeyFile, the ILRepack keyfile path incake-build/helpers/tools.cake,generate_key.bat, 4AssemblyInfocommentssrc/Ably.PubSub.Core.NETFramework/IO.Ably.snksrc/IO.Ably.rulesetsrc/Ably.PubSub.rulesetCodeAnalysisRuleSetelements across 8 csproj files (Ably.PubSub.Core,Ably.PubSub.Core.NETFramework×2,Ably.PubSub.Device,Ably.PubSub.Server,Ably.PubSub.Server.NETFramework×2,Ably.PubSub.Tests.DotNET,Ably.PubSub.Tests.NETFramework×2,examples/NotificationsPublisher; the commit message says "nine"), 6 solution-item entries across the 4 solutions, internalRuleSet Name/Descriptionmetadata,CLAUDE.mdsrc/IO.Ably.Tests.rulesetsrc/Ably.PubSub.Tests.rulesetAlso deleted:
src/Ably.PubSub/CustomSerialisers/GeneratedSerializers/IO/Ably/…— three MsgPack serializer files from a long-ago generator run, compiled by no project (verified: no projitems/csproj reference), in a directory whose path still spelled the old namespace. The only mention of that directory is theOutputDirectoryof a permanently skipped (Skip="true") generator-scaffold test; its path is deliberately left untouched — it is dead scenery that any resolution of the MSGPACK question (below) sweeps away.Signing: identity unchanged, verified
The key file's bytes are unchanged — only the name moves. Verified: the public key extracted from the renamed
Ably.PubSub.snk(sn -p/sn -tp) is byte-identical to thePublicKey=blob in theInternalsVisibleTogrants (token70c9e5c3d2c68b16), and Package-mode signed builds of the core and both door packages produce assemblies that pass strong-name verification. Assembly identities and every IVT grant are unaffected.A note on the
AssemblyInfocomments: a review fix on the namespace PR restored them to the then-trueIO.Ably.snk; this rename moves the file to what those comments always meant, so they now readAbly.PubSub.snkand stay accurate.Pre-existing defect found (not fixed here)
While validating, we found the
MSGPACKconditional-compile path does not compile at all — and the identical defect exists on 1.xmain, so it predates this campaign entirely (nothing definesMSGPACKin CI, so no build ever caught it). Five distinct error classes, evidence preserved:Defaults.Protocoldoes not exist in the#if MSGPACKarm (it definesDefaultProtocol, which nothing consumes).ProtocolMessageserializer still reads/writesConnectionSerial, which was removed from the protocol.MessageHandler._protocolis no longer static, but the arm uses it statically.List<Stats>/IEnumerable<T>conversion the arm never tracked.optionsandLogRequestBodysymbols that no longer exist.The wire-format drift (2) and structural drift (3) show the arm missed several protocol generations — this is beyond a hygiene fix. No fix ships in this PR; whether to remove or resurrect the MSGPACK path is a separate product decision. The 1.x line shares the defect (a deferred backport candidate if it is ever fixed).
What deliberately remains
IO.Ablycorrectly names the 1.x past (incl.IoC.cs's history comment).IO.Ably.DeltaCodecsubmodule: an external repository; renaming it is a separate cross-repo decision.IO_Ably_*MsgPack generated serializers — exactly 18IO_Ably_*files insrc/Ably.PubSub.Shared.MsgPack/CustomSerialisers/GeneratedSerializers/, their 18MsgPackHelperregistrations and their 18Ably.PubSub.Shared.MsgPack.projitemsentries, and nothing else: pending the MSGPACK-path decision above. They cannot be validated-renamed while the arm does not compile, and either resolution (removal or regeneration) eliminates them wholesale — renaming them now would be wasted work under every branch of that decision.Validation
./build.sh --target=Build.NetStandard(Release, StyleCop-as-error, against the renamed rulesets): 0 warnings / 0 errors./build.sh --target=Build.NetFrameworkunder mono (the net46 project's key path changed): greenPackage=True) of core and both doors: clean;sn -vvalid on the outputs; extracted-public-key vs IVT-blob diff: identicalSummary by CodeRabbit