Skip to content

Fix documentation paths, error codes, and script robustness#1644

Merged
clairernovotny merged 3 commits into
use-docfxfrom
copilot/sub-pr-1642
Nov 6, 2025
Merged

Fix documentation paths, error codes, and script robustness#1644
clairernovotny merged 3 commits into
use-docfxfrom
copilot/sub-pr-1642

Conversation

Copilot AI commented Nov 6, 2025

Copy link
Copy Markdown
Contributor

Addresses code review feedback on the DocFX migration PR by fixing broken documentation references, correcting error codes, and improving build script reliability.

Documentation fixes:

  • Replace relative path to UseCultureAttribute.cs with GitHub permalink to prevent broken links in generated DocFX site
  • Correct CONTRIBUTING.md link path from contributing.md to ../.github/CONTRIBUTING.md
  • Remove incorrect NU3004 error code (only NU1004 applies to locale parsing failures)
  • Fix version path pattern from /v{major.minor}/ to /{version}/ to match build script implementation
  • Add thread-safety guidance for custom caching using ConcurrentDictionary
  • Simplify ToQuantity example to use English "item" instead of French "élément" with clarifying comment

Build script improvements:

  • Guard array iteration in cleanup function to prevent "unbound variable" errors with set -u
  • Add docfx tool existence check with actionable error message before attempting builds

The changes ensure documentation links resolve correctly, remove confusion about error codes and pluralization behavior, and make the build script more robust against edge cases.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…mples

Co-authored-by: clairernovotny <1427284+clairernovotny@users.noreply.github.com>
Copilot AI changed the title [WIP] Migrate docs site from Jekyll to DocFX Fix documentation paths, error codes, and script robustness Nov 6, 2025
Copilot AI requested a review from clairernovotny November 6, 2025 03:49
@clairernovotny
clairernovotny marked this pull request as ready for review November 6, 2025 03:51
Copilot AI review requested due to automatic review settings November 6, 2025 03:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates documentation to improve clarity and correctness across troubleshooting, testing, performance, and versioning sections.

Key changes include:

  • Corrected error code references and documentation links
  • Enhanced shell script robustness with better array handling and tool verification
  • Added concurrency guidance for custom caching scenarios

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/troubleshooting.md Removed incorrect error code NU3004 and fixed contributing guide link path
docs/testing.md Converted relative repository link to absolute GitHub URL
docs/performance.md Added thread-safety note for custom caching and clarified code comment explaining ToQuantity behavior
docs/index.md Generalized version path description from /v{major.minor}/ to /{version}/
.github/workflows/scripts/build-docfx.sh Added array bounds check to prevent bash errors and added docfx tool existence validation

Comment thread docs/performance.md Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@clairernovotny
clairernovotny merged commit 4e8df74 into use-docfx Nov 6, 2025
6 checks passed
@clairernovotny
clairernovotny deleted the copilot/sub-pr-1642 branch November 6, 2025 03:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

# Verify docfx tool exists
if [ ! -f "$ROOT/docfx" ]; then
echo "Error: docfx tool not found at $ROOT/docfx"
echo "Please run: dotnet tool install --tool-path . docfx"

Copilot AI Nov 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message suggests installing docfx using --tool-path . which will install to the current directory. However, the check looks for docfx at $ROOT/docfx. If the script is run from a different directory than $ROOT, the suggestion won't match the check location. Consider suggesting dotnet tool restore instead (which uses .config/dotnet-tools.json) or clarify the working directory requirement.

Suggested change
echo "Please run: dotnet tool install --tool-path . docfx"
echo "Please run: dotnet tool install --tool-path \"$ROOT\" docfx"
echo "Or, if you have a dotnet tool manifest, run: dotnet tool restore"

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants