Skip to content

Conversation

@nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Jan 18, 2025

User description

Description

Updating tools to the latest and pin. Verified locally.

Motivation and Context

Fixes errors while generating public docs.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix, Enhancement


Description

  • Updated DocFX tool version to 2.78.2 for compatibility.

  • Removed outdated comments and references to older DocFX versions.

  • Adjusted docfx.json to comment out problematic outputFormat settings.

  • Simplified GitHub workflow by removing redundant DocFX installation step.


Changes walkthrough 📝

Relevant files
Configuration changes
update-documentation.yml
Simplify GitHub workflow for documentation updates             

.github/workflows/update-documentation.yml

  • Removed specific DocFX installation step from workflow.
  • Simplified GitHub workflow for documentation updates.
  • +0/-4     
    Enhancement
    Rakefile
    Update DocFX version in Rakefile                                                 

    Rakefile

  • Updated DocFX version from 2.75.3 to 2.78.2.
  • Removed outdated comments about older DocFX issues.
  • +2/-3     
    Bug fix
    docfx.json
    Adjust `docfx.json` to handle `outputFormat` issues           

    dotnet/docs/docfx.json

  • Commented out outputFormat settings causing errors.
  • Added notes about issues with apiPage generation.
  • +2/-3     

    Need help?
  • Type /help how to ... in the comments thread for any question about Qodo Merge usage.
  • Check out the documentation for more information.
  • @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Version Compatibility

    The update to DocFX 2.78.2 should be validated to ensure it doesn't introduce any breaking changes or documentation generation issues. Consider documenting any known limitations.

    # Pinning to 2.78.2 to avoid breaking changes in newer versions
    sh 'dotnet tool uninstall --global docfx || true'
    sh 'dotnet tool install --global --version 2.78.2 docfx'
    Documentation Quality

    Commenting out the outputFormat settings may affect the structure and quality of generated documentation. The apiPage generation errors should be investigated and properly resolved rather than disabled.

    //"outputFormat": "apiPage" // "apiPage" generation with errors

    @qodo-merge-pro
    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Add proper error handling for tool installation to prevent silent failures and improve debugging

    Add error handling for the DocFX installation command to gracefully handle network
    issues or installation failures. The current implementation might fail silently if
    the installation encounters problems.

    Rakefile [782]

    -sh 'dotnet tool install --global --version 2.78.2 docfx'
    +begin
    +  sh 'dotnet tool install --global --version 2.78.2 docfx'
    +rescue StandardError => e
    +  puts "Failed to install DocFX: #{e.message}"
    +  raise "DocFX installation failed. Please check your network connection and try again."
    +end
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion adds robust error handling for DocFX installation, which would help diagnose and troubleshoot installation issues. This is particularly important given the recent version upgrade from 2.75.3 to 2.78.2.

    7
    Improve configuration documentation by providing detailed context for disabled settings

    Instead of commenting out the outputFormat settings, it would be better to document
    the specific errors encountered and provide a clear explanation in the configuration
    file about why this setting is disabled.

    dotnet/docs/docfx.json [14]

    -//"outputFormat": "apiPage" // "apiPage" generation with errors
    +/* Temporarily disabled due to DocFX 2.78.2 compatibility issues:
    +   - Issue #XYZ: ApiPage generation fails with specific error message
    +   - Pending upstream fix in future DocFX release
    +*/
    +// "outputFormat": "apiPage"
    • Apply this suggestion
    Suggestion importance[1-10]: 3

    Why: While the suggestion aims to improve documentation, it only adds more verbose comments without addressing the underlying technical issue. The current comment is already sufficient to indicate the problem with apiPage generation.

    3

    @nvborisenko
    Copy link
    Member Author

    Thanks, hopefully we will see good docs published.

    @nvborisenko nvborisenko merged commit 9e72af6 into SeleniumHQ:trunk Jan 18, 2025
    24 checks passed
    @nvborisenko nvborisenko deleted the dotnet-docs-fix branch January 18, 2025 15:18
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Mar 23, 2025
    gryznar pushed a commit to gryznar/selenium that referenced this pull request May 17, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    1 participant