Conversation
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughReplaces legacy .sln solutions with .slnx manifests, updates SDK/tools and package versions, adjusts GitHub Actions to use global.json with caching, modifies build targets to build the .slnx solution, removes MSSolution package restore from setup, and adds PowerShell scripts for full local build and tool updates. Several formatting-only changes applied. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant PS as PowerShell (fullbuild.ps1)
participant DotNet as dotnet CLI
participant Setup as Build/Setup.fsproj
participant Build as Build/Build.fsproj
Dev->>PS: Run fullbuild.ps1
PS->>DotNet: dotnet tool restore
PS->>DotNet: dotnet run --project Setup
DotNet->>Setup: Execute Preparation
Setup-->>PS: Completed
PS->>DotNet: dotnet run --project Build
DotNet->>Build: Execute targets (uses altcode.dixon.slnx)
Build-->>PS: Build output
PS-->>Dev: Script finished (fail-fast on errors)
sequenceDiagram
autonumber
participant GH as GitHub Runner
participant CO as actions/checkout@v5
participant SD as actions/setup-dotnet@v4
participant Cache as NuGet Cache
participant Build as Build Pipeline
GH->>CO: Checkout repository
GH->>SD: Setup .NET (global.json)
SD->>Cache: Restore/prime cache (csproj/fsproj/packages.lock.json)
GH->>Build: Run existing steps (unchanged)
Build->>Build: Build targets referencing .slnx
Build-->>GH: Results/artifacts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Join our Discord community for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
.github/workflows/main.yml (1)
22-27: Dotnet setup via global.json + caching looks good; consider minimal permissionsUsing global-json-file with caching is correct. For least-privilege, set permissions at the workflow/job level.
Example minimal permission:
name: CI +permissions: + contents: read.config/dotnet-tools.json (1)
5-11: Validate the “rollForward”: false property in tool manifestThe added rollForward flags may not be recognized in all SDKs/tooling versions. If unrecognized, restore/update can fail or ignore the setting silently.
Please run in your environment/CI:
- dotnet --info
- dotnet tool restore
- dotnet tool list --tool-manifest ./.config/dotnet-tools.json
If issues occur, consider removing rollForward or replacing with the currently supported manifest field (e.g., rollForwardPolicy) per your SDK’s docs.
Also applies to: 13-18, 20-25
FakeForAltCodeDixonBuild.slnx (1)
1-1: Nit: consider adding an XML prolog for clarity/compatibilityWhile optional, an XML header can improve tooling interoperability and readability.
+<?xml version="1.0" encoding="utf-8"?> <Solution>altcode.dixon.slnx (3)
1-1: Nit: add an XML prologOptional, but can help some XML-aware tooling.
+<?xml version="1.0" encoding="utf-8"?> <Solution>
23-25: Confirm x86 platform requirement; consider AnyCPU or removing Platform for CI portabilityTargeting x86 may constrain builds on GitHub-hosted Linux/macOS runners and ARM hardware. If not strictly required, prefer AnyCPU or omit Platform to inherit defaults.
Options (choose one if appropriate):
- Change to AnyCPU:
- <Platform Project="x86" /> + <Platform Project="AnyCPU" />
- Or remove the Platform override entirely:
- <Platform Project="x86" />Also applies to: 26-29
2-16: Optional: align folder naming with the other solution for consistencyThis solution uses "/BuildItems/" whereas FakeForAltCodeDixonBuild.slnx uses "/Solution Items/". Consider a consistent naming convention across solutions.
- <Folder Name="/BuildItems/"> + <Folder Name="/Solution Items/">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
.config/dotnet-tools.json(1 hunks).github/workflows/main.yml(1 hunks)Build/actions.fs(1 hunks)Build/setup.fs(0 hunks)Build/targets.fs(6 hunks)Directory.Packages.props(3 hunks)FakeForAltCodeDixonBuild.sln(0 hunks)FakeForAltCodeDixonBuild.slnx(1 hunks)FixCop/Program.fs(1 hunks)ToolUpdate.ps1(1 hunks)altcode.dixon.sln(0 hunks)altcode.dixon.slnx(1 hunks)fsharplint.json(1 hunks)fullbuild.ps1(1 hunks)global.json(1 hunks)
💤 Files with no reviewable changes (3)
- Build/setup.fs
- FakeForAltCodeDixonBuild.sln
- altcode.dixon.sln
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: windows
🔇 Additional comments (14)
global.json (1)
3-3: CI workflows properly honor global.json and pin .NET SDKAll checks confirm:
.github/workflows/main.ymlusesactions/setup-dotnet@v4withglobal-json-file: global.json(line 22)- No
dotnet-version:overrides or hard-coded9.0.304actions/checkout@v5.0.0is in use, ensuring a supported runnerSDK 9.0.304 will be pulled from global.json as intended.
FixCop/Program.fs (1)
38-38: LGTM: formatting-only change is correctCollapsing the fluent call into a single line preserves behavior and keeps the intent clear.
fsharplint.json (1)
261-261: LGTM: disabling indentation rule without config is fineGiven the tool bump, removing the nested config and keeping
"enabled": falseis a reasonable simplification.Build/actions.fs (1)
19-19: LGTM: cosmetic reflow onlyThe single-line DirectoryInfo/GetDirectories expression is equivalent to the previous multi-line form.
.github/workflows/main.yml (1)
17-17: No self-hosted runners in workflows; v5 upgrade is safeI’ve checked .github/workflows/main.yml and only saw
runs-on: windows-latest—no self-hosted runners are configured. GitHub-hosted runners meet the v5 requirements, so you can safely upgrade to actions/checkout@v5.0.0.Build/targets.fs (5)
383-383: Restore glob change is cosmeticNo functional change; still restores all project types as intended.
431-432: Lint target consolidation is fineCombining glob patterns/readability improvement without changing behavior.
759-759: BulkReport glob tightening is benignCosmetic change; no impact on the filtered set.
780-780: Packaging glob reflow is benignNo functional change to the artifact publishing selection.
403-407: Confirm .slnx replacement – ensure MSBuild/VS support
- Verified:
altcode.dixon.slnxexists at the repo root- Verified: no
altcode.dixon.slnfile remains
Please ensure that all developer workstations and CI agents run an MSBuild/Visual Studio version that recognizes the.slnxformat. If any environments lag behind, consider adding an alternate build target that invokes the individual.fsprojfiles directly.Directory.Packages.props (1)
35-35: Confirm FSharp.Core 9.0.303 compatibility with all target frameworks (e.g., net472 consumers)If any projects still target .NET Framework and rely on netstandard2.0 assets, ensure the selected FSharp.Core assets satisfy binding requirements at runtime.
FakeForAltCodeDixonBuild.slnx (1)
1-18: LGTM: Clean slnx manifest with well-chosen Solution Items and build projectsThe structure is straightforward and aligns with the repo’s build/config layout. No functional concerns spotted in this manifest.
altcode.dixon.slnx (2)
1-31: LGTM: Comprehensive slnx capturing build tooling, main projects, and dependenciesThe layout is coherent: a BuildItems folder for tooling, explicit build toggles for non-default projects, and clear dependencies. Looks good.
17-22: Are tests intentionally excluded from the default build?Both test projects are marked Project="false". If CI should still run tests, ensure the workflow invokes them explicitly or uses a separate test solution/configuration.
Build/targets.fs
Outdated
| .ToString("HHmmss") | ||
| .Substring(0, 5) | ||
| .TrimStart('0') | ||
| now.ToString("HHmmss").Substring(0, 5).TrimStart('0') |
There was a problem hiding this comment.
Potential invalid assembly version at midnight due to TrimStart('0') producing an empty segment
If time is “00000” (e.g., 00:00:0x), TrimStart('0') yields "", producing a version like “Y.M.D.” which is invalid for AssemblyVersion/FileVersion.
Apply this to guarantee a numeric segment:
- now.ToString("HHmmss").Substring(0, 5).TrimStart('0')
+ let s = now.ToString("HHmmss").Substring(0, 5).TrimStart('0')
+ if String.IsNullOrEmpty s then "0" else s📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| now.ToString("HHmmss").Substring(0, 5).TrimStart('0') | |
| let s = now.ToString("HHmmss").Substring(0, 5).TrimStart('0') | |
| if String.IsNullOrEmpty s then "0" else s |
🤖 Prompt for AI Agents
In Build/targets.fs around line 325, the expression
now.ToString("HHmmss").Substring(0, 5).TrimStart('0') can produce an empty
string at midnight and yield an invalid assembly version segment; change the
logic so the trimmed result is converted to a numeric-safe segment (e.g., trim
leading zeros, then if the result is empty set it to "0" or parse to int and use
ToString()), ensuring the version segment is always a valid numeric string.
| <PackageVersion Include="NUnit" Version="4.4.0" /> | ||
| <PackageVersion Include="NUnit3TestAdapter" Version="4.3.0" /> | ||
| <PackageVersion Include="NUnit.ConsoleRunner" Version="3.16.3" /> | ||
| <PackageVersion Include="NUnit.ConsoleRunner" Version="3.20.1" /> | ||
| <PackageVersion Include="OpenCover" Version="4.7.1221" /> |
There was a problem hiding this comment.
NUnit 4 vs NUnit 3 infrastructure mismatch likely to break test execution
You upgraded NUnit to 4.4.0 but still use NUnit3TestAdapter and nunit3-console (ConsoleRunner 3.x). NUnit 3 adapters/runners don’t execute NUnit 4 tests.
Two paths:
- Revert to NUnit 3.x to keep the current infra (NUnit3TestAdapter + ConsoleRunner 3.x).
- Or migrate the test infra to NUnit 4 compatible tooling (adapter/console that supports v4).
If you prefer to revert for now, change these lines:
- <PackageVersion Include="NUnit" Version="4.4.0" />
+ <PackageVersion Include="NUnit" Version="3.13.3" />
<PackageVersion Include="NUnit3TestAdapter" Version="4.3.0" />
- <PackageVersion Include="NUnit.ConsoleRunner" Version="3.20.1" />
+ <PackageVersion Include="NUnit.ConsoleRunner" Version="3.20.1" />If you want to proceed with NUnit 4, I can propose the compatible adapter/runner set.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <PackageVersion Include="NUnit" Version="4.4.0" /> | |
| <PackageVersion Include="NUnit3TestAdapter" Version="4.3.0" /> | |
| <PackageVersion Include="NUnit.ConsoleRunner" Version="3.16.3" /> | |
| <PackageVersion Include="NUnit.ConsoleRunner" Version="3.20.1" /> | |
| <PackageVersion Include="OpenCover" Version="4.7.1221" /> | |
| <PackageVersion Include="NUnit" Version="3.13.3" /> | |
| <PackageVersion Include="NUnit3TestAdapter" Version="4.3.0" /> | |
| <PackageVersion Include="NUnit.ConsoleRunner" Version="3.20.1" /> | |
| <PackageVersion Include="OpenCover" Version="4.7.1221" /> |
🤖 Prompt for AI Agents
In Directory.Packages.props around lines 48-51, NUnit was bumped to 4.4.0 while
the project still references NUnit3TestAdapter and NUnit.ConsoleRunner 3.x
(NUnit 3 infra), which will prevent tests from running; either revert NUnit to a
3.x release that is compatible with the current adapter/console (e.g., set NUnit
to a 3.13.x stable version) or update the test infrastructure to NUnit
4-compatible tooling (replace NUnit3TestAdapter and ConsoleRunner 3.x with the
NUnit 4 adapter/engine and console packages that support v4); pick one path and
update the versions in this file so the framework and runners/adapters match.
| <File Path=".config/dotnet-tools.json" /> | ||
| <File Path=".github/workflows/main.yml" /> | ||
| <File Path="Build/build-rules.xml" /> | ||
| <File Path="Build/Directory.Build.props" /> | ||
| <File Path="Directory.Build.props" /> | ||
| <File Path="Directory.Build.targets" /> | ||
| <File Path="Directory.Packages.props" /> | ||
| <File Path="OutputBuildProps.props" /> | ||
| <File Path="OutputBuildTargets.props" /> | ||
| <File Path="README.md" /> | ||
| <File Path="ReleaseNotes - Previously.md" /> | ||
| <File Path="ReleaseNotes.md" /> | ||
| </Folder> | ||
| <Project Path="Build/Build.fsproj" /> | ||
| <Project Path="Build/Setup.fsproj" /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify referenced paths exist and that actions/checkout is bumped to v5 in workflows
This manifest references many files (including .github/workflows/main.yml). Let’s verify all referenced paths resolve and that workflows use actions/checkout@v5 and suitable runners.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking that all referenced .slnx paths exist..."
python - << 'PY'
import xml.etree.ElementTree as ET, glob, os, sys
slnx_files = glob.glob('**/*.slnx', recursive=True)
missing = []
for slx in slnx_files:
tree = ET.parse(slx)
root = tree.getroot()
for elem in root.iter():
if elem.tag in ('File','Project'):
p = elem.get('Path')
if p and not os.path.exists(p):
missing.append((slx, p))
if elem.tag == 'BuildDependency':
p = elem.get('Project')
if p and not os.path.exists(p):
missing.append((slx, p))
if missing:
print('Missing paths referenced in .slnx:')
for slx, p in missing:
print(f' {slx}: {p}')
sys.exit(1)
print('All referenced paths in .slnx exist.')
PY
echo
echo "Inspecting actions/checkout usage in workflows..."
if fd -t f -g '.github/workflows/*.yml' >/dev/null 2>&1; then
echo "All checkout usages (context lines shown):"
rg -nP --no-heading -C2 '\buses:\s*actions/checkout@[^ \t]+' .github/workflows || true
echo
echo "Checking for non-v5 usages of actions/checkout..."
if rg -nP --no-heading '\buses:\s*actions/checkout@(?!v5(?:\b|\.))\S+' .github/workflows >/dev/null; then
echo "Found non-v5 usages of actions/checkout:"
rg -nP --no-heading -C1 '\buses:\s*actions/checkout@(?!v5(?:\b|\.))\S+' .github/workflows
exit 2
else
echo "All actions/checkout usages are v5 (or none found)."
fi
echo
echo "Runner targets:"
rg -nP --no-heading '^\s*runs-on\s*:' .github/workflows || true
else
echo "No workflow files found at .github/workflows/*.yml"
fiLength of output: 330
Missing file references in altcode.dixon.slnx
The solution manifest is referencing Build/*.fsx files that don’t exist in the repo:
- Build/actions.fsx
- Build/build.fsx
- Build/setup.fsx
- Build/targets.fsx
These must either be added to version control or their <File> entries removed from altcode.dixon.slnx.
Once the manifest is corrected, also verify that your GitHub workflows in .github/workflows/*.yml:
- Use
actions/checkout@v5 - Specify appropriate
runs-onrunner targets
| Set-StrictMode -Version Latest | ||
| $ErrorActionPreference = 'Stop' | ||
| Set-Location -Path $PSScriptRoot | ||
|
|
||
| dotnet tool restore | ||
| dotnet run --project (Join-Path $PSScriptRoot 'Build/Setup.fsproj') | ||
| dotnet run --project (Join-Path $PSScriptRoot 'Build/Build.fsproj') No newline at end of file |
There was a problem hiding this comment.
Native command failures won’t stop the script; add exit-code checks
In PowerShell, external commands (like dotnet) returning non-zero exit codes don’t trigger terminating errors under ErrorActionPreference=Stop. The script will continue after failures, potentially masking build problems.
Apply this diff to fail fast on errors:
Set-StrictMode -Version Latest
$ErrorActionPreference = 'Stop'
Set-Location -Path $PSScriptRoot
dotnet tool restore
+if ($LASTEXITCODE -ne 0) { throw "dotnet tool restore failed with exit code $LASTEXITCODE" }
dotnet run --project (Join-Path $PSScriptRoot 'Build/Setup.fsproj')
+if ($LASTEXITCODE -ne 0) { throw "dotnet run Build/Setup.fsproj failed with exit code $LASTEXITCODE" }
dotnet run --project (Join-Path $PSScriptRoot 'Build/Build.fsproj')
+if ($LASTEXITCODE -ne 0) { throw "dotnet run Build/Build.fsproj failed with exit code $LASTEXITCODE" }🤖 Prompt for AI Agents
In fullbuild.ps1 around lines 1 to 7, external dotnet commands are invoked but
their non-zero exit codes are not checked, so failures won't stop the script;
after each dotnet invocation (dotnet tool restore and both dotnet run lines)
capture and test $LASTEXITCODE (or the process exit code) and if non-zero call
exit with that code (or throw) to fail fast; update the script to check
$LASTEXITCODE immediately after each external command and exit with that code
when it is not 0, ensuring build failures stop the script.
| & dotnet tool update dotnet-reportgenerator-globaltool | ||
| & dotnet tool update altcode.gendarme-tool | ||
| & dotnet tool update dotnet-fsharplint | ||
| & dotnet tool update -g fantomas No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make tool updates deterministic and fail fast; bind to the local tool manifest explicitly
As written, failures won’t stop the script, and updates depend on the current working directory implicitly containing the manifest. Tighten reliability and reproducibility.
Apply this diff to add strict error handling, pin to the local manifest, and keep the global fantomas update:
+Set-StrictMode -Version Latest
+$ErrorActionPreference = "Stop"
+
+Push-Location $PSScriptRoot
+try {
-& dotnet tool update dotnet-reportgenerator-globaltool
-& dotnet tool update altcode.gendarme-tool
-& dotnet tool update dotnet-fsharplint
-& dotnet tool update -g fantomas
+ & dotnet tool restore --tool-manifest .\.config\dotnet-tools.json
+ & dotnet tool update --tool-manifest .\.config\dotnet-tools.json dotnet-reportgenerator-globaltool
+ & dotnet tool update --tool-manifest .\.config\dotnet-tools.json altcode.gendarme-tool
+ & dotnet tool update --tool-manifest .\.config\dotnet-tools.json dotnet-fsharplint
+ & dotnet tool update -g fantomas
+}
+finally {
+ Pop-Location
+}🤖 Prompt for AI Agents
In ToolUpdate.ps1 lines 1-4, the script currently runs updates
non-deterministically and won’t stop on failures; change it to set PowerShell to
fail fast (e.g. $ErrorActionPreference = 'Stop' at the top) and update the local
tools by explicitly targeting the repository tool manifest (use the --local and
--tool-manifest <path> flags, e.g. --local --tool-manifest
.config/dotnet-tools.json) for the first three updates, while keeping the
fantomas update global (-g); ensure each dotnet invocation relies on those flags
so failures cause the script to exit immediately.
Bumps actions/checkout from 4 to 5.
Release notes
Sourced from actions/checkout's releases.
... (truncated)
Changelog
Sourced from actions/checkout's changelog.
... (truncated)
Commits
08c6903Prepare v5.0.0 release (#2238)9f26565Update actions checkout to use node 24 (#2226)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)Summary by CodeRabbit