chore: drop CI-detection & doc-file config already provided by ANcpLua.NET.Sdk#496
Conversation
…Lua.NET.Sdk The <!-- CI Detection --> PropertyGroup set ContinuousIntegrationBuild=true on GITHUB_ACTIONS/TF_BUILD — a strict subset of what the consumed ANcpLua.NET.Sdk already covers (ContinuousIntegrationBuild.props: GITHUB_ACTIONS, TF_BUILD, CI, and ~10 more). GenerateDocumentationFile=true likewise duplicated the SDK's Common.props value. Both removed; the SDK is the single source of build policy. Verified value-preserving and gate-safe: - CI-sim (GITHUB_ACTIONS=true, Release): ContinuousIntegrationBuild/ GenerateDocumentationFile/TreatWarningsAsErrors all unchanged vs before. - Local Debug: ContinuousIntegrationBuild empty and TreatWarningsAsErrors false before and after — the removed block never applied locally. - Gate still fires: planted #warning becomes error CS1030, fails the CI build. - Full solution build 0 warnings / 0 errors. Kept qyl's intentional overrides (AnalysisLevel 10.0 / AnalysisMode Recommended, the curated WarningsAsErrors CA-rule list). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary by CodeRabbit
WalkthroughDirectory.Build.props no longer sets GenerateDocumentationFile or the CI-based ContinuousIntegrationBuild conditional block directly; both are replaced with comments indicating these are now supplied by ANcpLua.NET.Sdk's shared props files. ChangesSDK delegation of build properties
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested labels: Two lines removed, two comments added — verify the SDK actually sets these properties before merging, otherwise builds silently lose doc generation and CI detection. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (7 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Directory.Build.props`:
- Around line 62-64: `ContinuousIntegrationBuild` is currently being enabled
from the generic `CI` environment variable, which can turn on
deterministic-build behavior in local shells and IDEs. Update the CI detection
in `Directory.Build.props` to rely on provider-specific variables only, and keep
the existing `ContinuousIntegrationBuild`/ANcpLua.NET.Sdk setup aligned with
those explicit CI signals rather than `CI` itself.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4957f28d-78eb-44f4-b430-877b429966a3
📒 Files selected for processing (1)
Directory.Build.props
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Dependency Audit
- GitHub Check: Backend (.NET)
- GitHub Check: Frontend (React)
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not suggest patterns that contradict
CLAUDE.mdorAGENTS.md.
Files:
Directory.Build.props
**
⚙️ CodeRabbit configuration file
AGENTS.md
**:
10.0.9
10.7.0
10.7.0
1.16.0
1.15.1
1.5.3
3.35.1
2.80.0
2.81.1
2.2.33
1.11.0<!-- MAF (Microsoft.Agents.AI.*) — mirrors SDK truth; inlined so the repo builds with or without the sibling SDK checkout. --> <MicrosoftAgentsAIVersion>1.11.0</MicrosoftAgentsAIVersion> <MicrosoftAgentsAIHostingVersion>1.11.0-preview.260623.1</MicrosoftAgentsAIHostingVersion> <NukeVersion>10.1.0</NukeVersion> <SpectreConsoleVersion>0.57.0</SpectreConsoleVersion> <VersionizeVersion>2.5.0</VersionizeVersion>
**:<Import Project="$(MSBuildThisFileDirectory)../framework/ANcpLua.NET.Sdk/src/Build/Common/Version.props"
...
Files:
Directory.Build.props
**/*.props
⚙️ CodeRabbit configuration file
MSBuild property files (Directory.Build.props, Directory.Packages.props, Version.props). Review for: Central Package Management correctness, version consistency, and that new packages are added with explicit version pins. Flag transitive dependency promotions that aren't justified. Verify TFM targeting is correct (.NET 10).
Files:
Directory.Build.props
🔇 Additional comments (1)
Directory.Build.props (1)
38-42: 🎯 Functional CorrectnessKeep
GenerateDocumentationFileon a shared source of truth.Directory.Build.propsnow relies onANcpLua.NET.Sdkto supply it, so this change is only safe if that SDK setsGenerateDocumentationFile=truefor every import path.
| <!-- CI detection (ContinuousIntegrationBuild) is provided by ANcpLua.NET.Sdk | ||
| (ContinuousIntegrationBuild.props) covering GITHUB_ACTIONS, TF_BUILD, CI and ~10 more. --> | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Best-effort: search for any local copy/reference of ContinuousIntegrationBuild.props
fd -HI 'ContinuousIntegrationBuild.props'
rg -n 'ContinuousIntegrationBuild' -g '*.props' -g '*.targets' -g '*.yml' -g '*.yaml'Repository: ANcpLua/qyl
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Directory.Build.props context:"
nl -ba Directory.Build.props | sed -n '50,75p'
echo
echo "Any CI/ContinuousIntegrationBuild references in repo:"
rg -n --hidden --glob '!**/.git/**' 'ContinuousIntegrationBuild|(^|[^A-Za-z])CI([^A-Za-z]|$)|GITHUB_ACTIONS|TF_BUILD' .Repository: ANcpLua/qyl
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import os, pathlib, re
for p in [pathlib.Path('Directory.Build.props')]:
if p.exists():
print(p, 'exists')
txt = p.read_text(encoding='utf-8', errors='replace').splitlines()
for i, line in enumerate(txt, 1):
if 58 <= i <= 70:
print(f"{i:4}: {line}")
else:
print(p, 'missing')
PYRepository: ANcpLua/qyl
Length of output: 727
🌐 Web query:
MSBuild ContinuousIntegrationBuild CI environment variable local tools false positive Directory.Build.props ANcpLua.NET.Sdk ContinuousIntegrationBuild.props
💡 Result:
The ContinuousIntegrationBuild MSBuild property is designed to enable deterministic builds, which normalize file paths in generated PDBs to ensure build reproducibility [1][2][3]. Because these normalized paths do not point to the developer's local machine, setting this property to true during local development will cause debuggers to fail to locate source files [1][4]. To avoid this issue, the property should be set conditionally in your Directory.Build.props file, ensuring it is enabled only when running in a CI environment [1][5][4]. Recommended Implementation in Directory.Build.props: Use the environment variables specific to your CI provider to trigger the setting [1][5][4]. For Azure Pipelines: true For GitHub Actions: true The term false positive in this context typically refers to static analysis tools or build analyzers that may trigger alerts (such as Proj0042) because they detect that ContinuousIntegrationBuild is not globally set to true [5][6]. If you have manually defined it in a conditional PropertyGroup in Directory.Build.props, ensure your analyzer or build process is correctly evaluating those MSBuild conditions [6]. Additionally, some users have reported issues where the property is too generic or conflicts with other tools, necessitating careful configuration to ensure it only activates when genuinely in a CI pipeline [7][8].
Citations:
- 1: https://github.com/clairernovotny/DeterministicBuilds/blob/master/README.md
- 2: The ContinuousIntegrationBuild csproj property is not documented dotnet/docs#22280
- 3: https://github.com/clairernovotny/DeterministicBuilds
- 4: https://bartwullems.blogspot.com/2021/01/improve-debugging-experience-with.html
- 5: https://dotnet-project-file-analyzers.github.io/rules/Proj0042
- 6: Execute conditions dotnet-project-file-analyzers/dotnet-project-file-analyzers#33
- 7: The name of /p:ContinuousIntegrationBuild is too generic dotnet/sdk#24232
- 8: Links to code for failed test 404 when deterministic builds are used Tyrrrz/GitHubActionsTestLogger#21
Keep ContinuousIntegrationBuild off generic CI. Local shells and IDEs often export CI, so this can enable deterministic-build mode outside real CI; gate it on provider-specific env vars instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Directory.Build.props` around lines 62 - 64, `ContinuousIntegrationBuild` is
currently being enabled from the generic `CI` environment variable, which can
turn on deterministic-build behavior in local shells and IDEs. Update the CI
detection in `Directory.Build.props` to rely on provider-specific variables
only, and keep the existing `ContinuousIntegrationBuild`/ANcpLua.NET.Sdk setup
aligned with those explicit CI signals rather than `CI` itself.
2 participants
Summary
Single-source-of-truth cleanup, matching the same change in ANcpLua.Analyzers#193. Removes two
Directory.Build.propssettings that merely re-state what the consumed ANcpLua.NET.Sdk already provides:<!-- CI Detection -->PropertyGroup (ContinuousIntegrationBuild=trueonGITHUB_ACTIONS/TF_BUILD) — a strict subset of the SDK'sContinuousIntegrationBuild.props(GITHUB_ACTIONS, TF_BUILD,CI, +~10 more).GenerateDocumentationFile=true— identical to the SDK'sCommon.propsvalue.Kept qyl's intentional overrides:
AnalysisLevel=10.0/AnalysisMode=Recommended(narrower than the SDK'slatest-all) and the curated always-onWarningsAsErrorsCA-rule list.What it changes: nothing (verified)
GITHUB_ACTIONS=true, Release):ContinuousIntegrationBuild=true,GenerateDocumentationFile=true,TreatWarningsAsErrors=true— identical before/after (dotnet build -getProperty).ContinuousIntegrationBuildempty andTreatWarningsAsErrors=falseboth before and after — the removed block was conditional and never applied locally.#warningbecomeserror CS1030and fails the CI-sim-c Releasebuild.Status: complete-and-verified.
🤖 Generated with Claude Code