Release v1.7.3 — visible web viewer version#258
Conversation
Reads PlanViewer.Web assembly version at runtime and renders it in two places so Joe (and anyone else giving feedback) can tell at a glance what version is deployed: - Landing page: small badge appended to the "Powered by Performance Studio" line - Toolbar (when a plan is loaded): badge on the right side next to the SQL Server build info deploy-web.yml now auto-syncs src/PlanViewer.Web/PlanViewer.Web.csproj Version with src/PlanViewer.App/PlanViewer.App.csproj Version before publishing, so the displayed version always matches the App release tag without needing manual csproj bumps in two places. Also widened the deploy-web path trigger to include App.csproj so version-only releases still redeploy the web. Version bump 1.7.2 -> 1.7.3. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
erikdarlingdata
left a comment
There was a problem hiding this comment.
Summary
v1.7.3 release PR: bumps PlanViewer.App version, adds a visible version badge to the web viewer's landing page and toolbar, and teaches deploy-web.yml to auto-sync the web csproj version from the app csproj (plus adds the app csproj to the web deploy's path triggers).
Base branch
Base is main, not dev. Flagging per repo convention — this is clearly a release merge (dev → main) so it's probably intentional, but worth confirming you meant to merge dev forward rather than land this on dev first.
What's good
- Path trigger for
PlanViewer.App.csprojfixes the actual problem: version-only bumps were not redeploying the web viewer, so the displayed version would have lagged. - Single source of truth (App csproj) with a CI-side sync is the right shape — avoids the double-bump drift the web csproj was already showing (
1.4.0vs1.7.2). AppVersioncomputed in astatic readonlywith a private helper — cheap, no per-render reflection.- No Avalonia / PlanAnalyzer /
src/logic changes, so no Dashboard/Lite sync or test-coverage concerns apply.
Needs attention
- CI sync step is fragile: XML access via
.Project.PropertyGroup.Versionand the unanchored regex replacement both break in plausible future csproj shapes. Inline comments ondeploy-web.ymllines 46 and 49. - Version display via
AssemblyName.Versionsilently falls out of sync if anyone later sets<AssemblyVersion>explicitly.AssemblyInformationalVersionis more robust. Inline comment onIndex.razor:1671. - Badge contrast: both new CSS classes use
--text-muted(#999999), and.toolbar-app-versionlayers it on a faint dark tint. Given the badge exists specifically to be readable at a glance, consider a higher-contrast color or a stronger pill background. Inline comment onapp.css:454. - Stale committed value in
PlanViewer.Web.csproj: now that CI overwrites it, flag it in-tree so a future bumper doesn't chase it by hand.
Comments only — not approving, not requesting changes.
Generated by Claude Code
| $appVersion = ([xml](Get-Content src/PlanViewer.App/PlanViewer.App.csproj)).Project.PropertyGroup.Version | Where-Object { $_ } | ||
| Write-Host "App version: $appVersion" | ||
| $webCsproj = 'src/PlanViewer.Web/PlanViewer.Web.csproj' | ||
| $content = Get-Content $webCsproj -Raw |
There was a problem hiding this comment.
.Project.PropertyGroup.Version is fragile: when a csproj has multiple <PropertyGroup> blocks (e.g. a Debug/Release conditional group), PropertyGroup becomes an array and .Version returns an array of values. Where-Object { $_ } filters empty strings but not duplicates — if a future PropertyGroup also declares <Version>, $appVersion becomes an array and the regex replacement below interpolates something like System.Object[] into the web csproj. Consider Select-Xml -XPath '/Project/PropertyGroup/Version' | Select-Object -First 1 -ExpandProperty Node | ForEach-Object InnerText or similar, and assert a single scalar result before proceeding.
Generated by Claude Code
| $content = Get-Content $webCsproj -Raw | ||
| $content = [regex]::Replace($content, '<Version>[^<]+</Version>', "<Version>$appVersion</Version>") | ||
| Set-Content -Path $webCsproj -Value $content -NoNewline | ||
| Write-Host "Synced web csproj to $appVersion" |
There was a problem hiding this comment.
The regex <Version>[^<]+</Version> is unanchored, so if a <PackageReference ... Version="..."> ever gets rewritten to the child-element form (<PackageReference ...><Version>...</Version></PackageReference>), or any other <Version> element is added, this replaces all of them. Safer to anchor inside a PropertyGroup or load the csproj as XML and set the node directly, matching how $appVersion is read.
Generated by Claude Code
| <Nullable>enable</Nullable> | ||
| <RootNamespace>PlanViewer.Web</RootNamespace> | ||
| <Version>1.4.0</Version> | ||
| <Version>1.7.3</Version> |
There was a problem hiding this comment.
Now that the workflow rewrites this value at publish time, the committed value is purely decorative. Worth a one-line comment here (or in the sync step) noting that the source of truth is PlanViewer.App.csproj so the next person bumping the app version doesn't also update this by hand and get confused when CI still overwrites it.
Generated by Claude Code
| private static readonly string AppVersion = GetAppVersion(); | ||
| private static string GetAppVersion() | ||
| { | ||
| var v = typeof(Index).Assembly.GetName().Version; |
There was a problem hiding this comment.
Assembly.GetName().Version returns the AssemblyVersion, which defaults to Major.Minor.Build.0 when only <Version> is set — so 1.7.3 → 1.7.3.0 and Major.Minor.Build renders 1.7.3 correctly. Just note that if anyone later sets <AssemblyVersion> explicitly (e.g. 1.0.0.0 for binary-compat reasons, common in libraries), this falls out of sync with the displayed version. AssemblyInformationalVersionAttribute (read via Assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion) tracks <Version> directly and is the more robust source for user-facing version strings.
Generated by Claude Code
| .toolbar-app-version { | ||
| font-size: 0.75rem; | ||
| font-weight: 600; | ||
| color: var(--text-muted); | ||
| padding: 0.1rem 0.4rem; | ||
| border-radius: 3px; | ||
| background: rgba(0, 0, 0, 0.05); |
There was a problem hiding this comment.
Both badges bind color: var(--text-muted), which resolves to #999999 (app.css:12). That's in the dim-grey range the desktop side explicitly rejected (ForegroundMutedBrush was moved to #B0B6C0 for readability). .toolbar-app-version additionally puts that grey on a rgba(0,0,0,0.05) tint, which reduces contrast further on light toolbars. Not introduced by this PR — the variable is already widespread — but the new badges are intentionally a "reviewers need to see the version at a glance" affordance, so it's worth either bumping them up (e.g. closer to --text-secondary/primary) or giving the pill a stronger background. Otherwise the thing that's supposed to be visible is the hardest-to-read label on the page.
Generated by Claude Code
Summary
See PR #257.
Test plan
🤖 Generated with Claude Code