Remove Newtonsoft.Json — migrate to System.Text.Json.Nodes#2
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
During the SAM-BIM Newtonsoft.Json -> System.Text.Json migration the quarterly sow/2026-Q2 branch carries the binary-breaking change. CI needs to consume the migrated SAM (and any sibling dep) from sow/2026-Q2, not from master, until the quarter-end merge. - Add "sow/2026-Q2" to push/pull_request branch triggers - For each dep clone, ls-remote sow/2026-Q2 and prefer it when present; fall back to default branch (e.g. master) otherwise. After the quarter merges back to master, this fallback naturally restores prior behaviour. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Refines the previous workflow patch so CI on the migration PR can succeed before anything has been merged. Each dep repo is cloned from: 1. github.head_ref (feature/remove-newtonsoft on these PRs) - has the migration right now on every dep 2. sow/2026-Q2 - source of truth after these PRs merge 3. default branch - source of truth after quarter-end merge Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mirrors the fix already applied on SAM_Revit / SAM_Revit_UI: this workflow also clones the private SAM_Solver dep, so the cascade ls-remote and clone must use the token-prefixed URL when $token is defined, otherwise GitHub prompts for credentials and CI fails with 'Cannot prompt because user interactivity has been disabled'. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The harmonized spdx-check workflow requires both header lines in the top 80 lines of every changed .cs file. SAM_UI was missed in the earlier fix-spdx pass; adding the header to the 45 files in the migration diff (excluding Properties/ and *.xaml.cs which the workflow skips). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The earlier fix_spdx pass detected only the en-dash form of the copyright line. Files that already had a hyphen-minus header got a second header pair prepended above the original instead of being recognised as already-compliant. Strip the duplicate, keeping the first (en-dash) pair. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ZiolkowskiJakub
approved these changes
May 18, 2026
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.
Summary
Migrates this SAM-BIM downstream repo from Newtonsoft.Json to System.Text.Json.Nodes to track the SAM core migration in SAM-BIM/SAM#3.
<PackageReference Include="Newtonsoft.Json">is replaced with<PackageReference Include="System.Text.Json" Version="10.0.8" />(matches SAM.Core's pinned version)JObject/JArray/JToken/JValue→JsonObject/JsonArray/JsonNode/JsonValueFromJObject(JObject)/JObject ToJObject()→FromJsonObject(JsonObject)/JsonObject ToJsonObject().Value<T>(key)→["key"]?.GetValue<T>() ?? default(T)foreach (JObject in JArray)rewritten asforeach (JsonNode) { if (!(node is JsonObject)) continue; … }(C# 7.3 compatible)JsonConvert.DeserializeObject<JObject>(s)/JObject.Parse(s)→JsonNode.Parse(s) as JsonObjectVerification
BuildAlls_v3 RestoreCleanRebuildacross the full SAM-BIM dependency chain (SAM → … → SAM_Revit_UI Debug2025/2026): 0 errors, 144 artifacts.JObject/JArray/JToken/JValue/ToJObject/FromJObject) remain in this repo.Test plan
🤖 Generated with Claude Code