Conversation
📝 WalkthroughWalkthroughThis PR bumps the DocFX base image version in the build Dockerfile, enhances the NuGet package bumping script with NuGet API version fetching and caching for Codebelt packages, and updates the Microsoft.NET.Test.Sdk dependency version. Changes
Possibly Related PRs
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR enhances CI/service-update automation by updating the DocFX build image, improving the repository’s NuGet package bumping automation, and bumping a test SDK dependency in the centrally-managed package versions file.
Changes:
- Bump
Microsoft.NET.Test.SdkinDirectory.Packages.props. - Improve
.github/scripts/bump-nuget.pyto set triggered packages to the provided version and update other Codebelt-related packages to latest stable from NuGet. - Update the DocFX Dockerfile to use
codebeltnet/docfx:2.78.5.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
Directory.Packages.props |
Updates centrally-managed package version for Microsoft.NET.Test.Sdk. |
.github/scripts/bump-nuget.py |
Expands Codebelt package mapping and adds NuGet “latest stable” fetching for non-triggered Codebelt packages. |
.docfx/Dockerfile.docfx |
Bumps the DocFX build image tag used for docs generation. |
| target_version = TRIGGER_VERSION.lstrip("v") | ||
|
|
||
| print(f"Trigger: {TRIGGER_SOURCE} @ {target_version}") | ||
| print(f"Only updating packages from: {TRIGGER_SOURCE}") | ||
| print(f"Triggered packages set to {target_version}; other Codebelt packages fetched from NuGet.") | ||
| print() |
There was a problem hiding this comment.
If TRIGGER_SOURCE is misspelled/unknown (not a key in SOURCE_PACKAGE_MAP), is_triggered_package never matches and the script will not pin any packages to TRIGGER_VERSION; instead it will treat all matching prefixes as “other Codebelt packages” and bump them to latest-from-NuGet. This can silently produce the wrong service update. Consider validating TRIGGER_SOURCE against SOURCE_PACKAGE_MAP (fail fast with a list of allowed values) before proceeding.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/bump-nuget.py (1)
100-107:⚠️ Potential issue | 🟠 MajorFail fast for unsupported
TRIGGER_SOURCE.If
TRIGGER_SOURCEis misspelled or unmapped, no package is treated as triggered, and the script may still bump many packages via the non-triggered Codebelt path.Suggested fix
def main(): if not TRIGGER_SOURCE or not TRIGGER_VERSION: print( "Error: TRIGGER_SOURCE and TRIGGER_VERSION environment variables required" ) print(f" TRIGGER_SOURCE={TRIGGER_SOURCE}") print(f" TRIGGER_VERSION={TRIGGER_VERSION}") sys.exit(1) + if TRIGGER_SOURCE not in SOURCE_PACKAGE_MAP: + print(f"Error: Unsupported TRIGGER_SOURCE '{TRIGGER_SOURCE}'") + print(f" Supported values: {', '.join(sorted(SOURCE_PACKAGE_MAP.keys()))}") + sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/bump-nuget.py around lines 100 - 107, Add a strict validation step for TRIGGER_SOURCE immediately after reading the environment variables (where TRIGGER_SOURCE and TRIGGER_VERSION are currently checked) to fail fast on misspelled or unmapped sources: verify TRIGGER_SOURCE exists in the script's mapping of supported trigger sources (e.g., the dict/constant that maps source names to package triggers) and, if not present, print a clear error showing the invalid value plus the list of supported values and call sys.exit(1); keep the existing TRIGGER_VERSION check and printing behavior but ensure the unsupported-source check runs before any package-bumping logic that assumes a valid mapping.
🧹 Nitpick comments (1)
.github/scripts/bump-nuget.py (1)
176-176: Simplify constant return expression.
return 0 if changes else 0is equivalent toreturn 0.Suggested cleanup
- return 0 if changes else 0 # Return 0 even if no changes (not an error) + return 0 # Return 0 even if no changes (not an error)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/bump-nuget.py at line 176, The return expression `return 0 if changes else 0` is redundant; update the function to simply `return 0` (remove the conditional) where that statement appears (referencing the `changes` variable and the current return in bump-nuget.py).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/bump-nuget.py:
- Around line 89-90: The current selection logic for latest stable (variables
"stable", "versions", and "result") incorrectly falls back to a prerelease when
no stable versions exist; change the assignment so "result" is the latest stable
if any (stable[-1]) and otherwise is None (do not return versions[-1] or any
prerelease), and update any callers of the function to handle a None return if
necessary.
---
Outside diff comments:
In @.github/scripts/bump-nuget.py:
- Around line 100-107: Add a strict validation step for TRIGGER_SOURCE
immediately after reading the environment variables (where TRIGGER_SOURCE and
TRIGGER_VERSION are currently checked) to fail fast on misspelled or unmapped
sources: verify TRIGGER_SOURCE exists in the script's mapping of supported
trigger sources (e.g., the dict/constant that maps source names to package
triggers) and, if not present, print a clear error showing the invalid value
plus the list of supported values and call sys.exit(1); keep the existing
TRIGGER_VERSION check and printing behavior but ensure the unsupported-source
check runs before any package-bumping logic that assumes a valid mapping.
---
Nitpick comments:
In @.github/scripts/bump-nuget.py:
- Line 176: The return expression `return 0 if changes else 0` is redundant;
update the function to simply `return 0` (remove the conditional) where that
statement appears (referencing the `changes` variable and the current return in
bump-nuget.py).
| stable = [v for v in versions if "-" not in v] | ||
| result = stable[-1] if stable else (versions[-1] if versions else None) |
There was a problem hiding this comment.
Do not fall back to prerelease when “latest stable” is required.
Current logic can select a prerelease when no stable version exists, which conflicts with the function contract and PR objective.
Suggested fix
- stable = [v for v in versions if "-" not in v]
- result = stable[-1] if stable else (versions[-1] if versions else None)
+ stable = [v for v in versions if "-" not in v]
+ result = stable[-1] if stable else None
+ if versions and not stable:
+ print(
+ f" Warning: No stable release found for {package_name}; keeping current version."
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/scripts/bump-nuget.py around lines 89 - 90, The current selection
logic for latest stable (variables "stable", "versions", and "result")
incorrectly falls back to a prerelease when no stable versions exist; change the
assignment so "result" is the latest stable if any (stable[-1]) and otherwise is
None (do not return versions[-1] or any prerelease), and update any callers of
the function to handle a None return if necessary.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #23 +/- ##
=======================================
Coverage 88.50% 88.50%
=======================================
Files 11 11
Lines 200 200
Branches 8 8
=======================================
Hits 177 177
Misses 23 23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This pull request updates the Dockerfile for DocFX, makes significant improvements to the NuGet package bumping script, and updates a test SDK dependency. The main focus is on enhancing the
.github/scripts/bump-nuget.pyscript to handle Codebelt-related package updates more intelligently, including fetching the latest stable versions for all Codebelt packages not directly triggered. Additionally, the script now supports a broader set of Codebelt packages and improves output clarity.NuGet package bumping script improvements:
.github/scripts/bump-nuget.pyto update all Codebelt-related packages to the latest stable version from NuGet, not just those from the triggering source. The script now distinguishes between triggered packages (set to the specified version) and other Codebelt packages (set to latest NuGet version), while still skipping third-party packages. [1] [2] [3] [4] [5]SOURCE_PACKAGE_MAPto include additional Codebelt packages, such asCodebelt.Extensions.Carter,Codebelt.Extensions.AspNetCore.Newtonsoft.Json,Codebelt.Extensions.AspNetCore.Text.Yaml, andCodebelt.SharedKernel, ensuring broader coverage.Dependency and base image updates:
codebeltnet/docfx:2.78.5instead of2.78.4.Microsoft.NET.Test.Sdkfrom version18.0.1to18.3.0inDirectory.Packages.props.Summary by CodeRabbit