Skip to content

Conversation

gimlichael
Copy link
Member

@gimlichael gimlichael commented Nov 11, 2024

This pull request includes several updates across multiple files to improve compatibility with .NET 9, update dependencies, and fix bugs. The most important changes include updates to package references, bug fixes in YAML formatters, and updates to release notes and changelog.

Updates to package references:

Bug fixes in YAML formatters:

  • PackageReleaseNotes.txt and CHANGELOG.md: Fixed the AddYamlFormattersOptions method to correctly pass the setup delegate to AddYamlExceptionResponseFormatter. [1] [2]

Updates to release notes and changelog:

Miscellaneous:

  • README.md: Updated the contributions section to include a link to the CONTRIBUTING.md file.
  • Various .csproj files: Removed specific versions from PackageReference elements to align with centralized version management. [1] [2] [3] [4] [5]

Summary by CodeRabbit

  • New Features

    • Added support for .NET 8 and .NET 9.
    • Introduced a new method for handling problem details in YAML formatters.
  • Bug Fixes

    • Improved exception handling and formatting for YAML responses.
    • Fixed setup delegate passing in YAML formatter options.
  • Documentation

    • Updated README for clarity and added contributing guidelines.
  • Chores

    • Removed specific version constraints for several package references to allow for the latest versions.

@gimlichael gimlichael self-assigned this Nov 11, 2024
Copy link

coderabbitai bot commented Nov 11, 2024

Walkthrough

The pull request introduces version 9.0.0 of the Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml package, removing support for .NET 6 while adding support for .NET 8 and .NET 9. Key updates include new methods in the YamlConverterExtensions class, modifications to existing methods to accommodate additional parameters, and fixes to ensure proper functionality of YAML formatters. The project files have been updated to remove specific version numbers from package references, promoting the use of the latest versions. Documentation has been slightly revised for clarity.

Changes

File Change Summary
.nuget/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml/PackageReleaseNotes.txt Version 9.0.0 updates: removed .NET 6 support, added .NET 8/9 support, new method AddProblemDetailsConverter, updated method signatures for AddHttpExceptionDescriptorConverter and AddYamlExceptionResponseFormatter, bug fixes for AddYamlFormattersOptions.
CHANGELOG.md Updated for version 9.0.0, highlighting major changes, new methods, breaking changes, and bug fixes.
Directory.Build.props Removed version specifications for several package references, allowing for latest version usage.
Directory.Packages.props New file created for centralized package version management.
README.md Minor textual updates for clarity and added hyperlink to contributing guidelines.
src/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml.csproj Removed version specification for Cuemon.AspNetCore.Mvc package reference.
src/Codebelt.Extensions.AspNetCore.Text.Yaml/Codebelt.Extensions.AspNetCore.Text.Yaml.csproj Removed version specifications for Cuemon.AspNetCore and Cuemon.Extensions.DependencyInjection.
src/Codebelt.Extensions.YamlDotNet/Codebelt.Extensions.YamlDotNet.csproj Removed version specifications for Cuemon.Extensions.Reflection, Cuemon.Extensions.IO, and YamlDotNet.
test/Codebelt.Extensions.AspNetCore.Text.Yaml.Tests/Codebelt.Extensions.AspNetCore.Text.Yaml.Tests.csproj Removed version specifications for Cuemon.Extensions.AspNetCore.Authentication, Cuemon.Extensions.AspNetCore, and Codebelt.Extensions.Xunit.Hosting.AspNetCore.
test/Codebelt.Extensions.YamlDotNet.Tests/Codebelt.Extensions.YamlDotNet.Tests.csproj Removed version specifications for System.Net.Http, Cuemon.Extensions.Globalization, and Cuemon.Extensions.Core.
test/Codebelt.Extensions.YamlDotNet.Tests/YamlSerializerTest.cs Updated expected output strings for serialization tests to accommodate framework-specific formatting and additional checks.

Possibly related PRs

  • V9.0.0/net9rc1 support #2: The changes in this PR include the removal of support for .NET 6 and the addition of support for .NET 8 and .NET 9, which directly aligns with the main PR's updates regarding framework support.
  • V9.0.0/dotnet rc2 #5: This PR also discusses the removal of support for .NET 6 and the introduction of new features and breaking changes, which are relevant to the updates made in the main PR regarding the YamlConverterExtensions class and method modifications.

🐰 In the meadow, changes bloom,
New versions chase away the gloom.
With YAML's charm, we now embrace,
.NET 8 and 9, a joyful space!
Bugs fixed, features bright and new,
Contributions welcome, come join the crew! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (6)
.nuget/Codebelt.Extensions.AspNetCore.Text.Yaml/PackageReleaseNotes.txt (1)

6-7: Consider adding a dedicated "Breaking Changes" section.

While the removal of .NET 6 support is documented under "ALM", it would be more visible and follow common practice to have it under a dedicated "Breaking Changes" section, especially since this is a major version release.

Consider restructuring like this:

 # ALM
- CHANGED Dependencies to latest and greatest with respect to TFMs
+ CHANGED Dependencies to latest and greatest with respect to TFMs
+
+ # Breaking Changes
 - REMOVED Support for TFM .NET 6 (LTS)
.nuget/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml/PackageReleaseNotes.txt (2)

8-10: LGTM! Consider adding more details about API changes.

The bug fixes are well documented. However, consider adding the following significant changes mentioned in the PR:

  • New method AddProblemDetailsConverter in YamlConverterExtensions
  • Updated signature of AddHttpExceptionDescriptorConverter to support Instance and TraceId
  • Modified AddYamlExceptionResponseFormatter to support ProblemDetails

Line range hint 1-24: Consider standardizing the release notes format.

A few suggestions to improve consistency:

  1. Add the release date (November 13, 2024)
  2. Use consistent heading styles (e.g., "Bug Fixes" vs "ALM")
  3. Group changes by type (Breaking Changes, New Features, Bug Fixes, etc.)
🧰 Tools
🪛 LanguageTool

[style] ~5-~5: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...GED Dependencies to latest and greatest with respect to TFMs

  • REMOVED Support for TFM .NET 6 ...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)

CHANGELOG.md (3)

Line range hint 14-17: Consider adding more context about the new methods.

While the additions are clearly documented, it would be helpful to briefly explain:

  • The purpose of AddFailureConverter
  • The use case for AddProblemDetailsConverter
🧰 Tools
🪛 LanguageTool

[style] ~8-~8: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...er.

[!NOTE]
Changelog entries prior to version 8.4.0 was migrated from previou...

(EN_WORDINESS_PREMIUM_PRIOR_TO)


[uncategorized] ~12-~12: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... introduced with .NET 9 preview releases so the final release is production ready t...

(COMMA_COMPOUND_SENTENCE_2)


Line range hint 19-27: Consider adding migration guidance for breaking changes.

The breaking changes are well documented, but users would benefit from:

  • Migration steps for the new camelCase naming convention
  • Examples of updating code for the new YamlFormatter parameter requirement
  • Guidelines for transitioning from FormatterOptions to Formatter property

28-31: Consider adding context about the fixes' impact.

While the fixes are documented, it would be helpful to:

  • Explain what issues these fixes resolve
  • Describe the impact of incorrect delegate passing
  • Note if any specific versions were affected by these issues
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 261de45 and b76591d.

📒 Files selected for processing (12)
  • .nuget/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml/PackageReleaseNotes.txt (1 hunks)
  • .nuget/Codebelt.Extensions.AspNetCore.Text.Yaml/PackageReleaseNotes.txt (1 hunks)
  • CHANGELOG.md (2 hunks)
  • Directory.Build.props (2 hunks)
  • Directory.Packages.props (1 hunks)
  • README.md (1 hunks)
  • src/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml.csproj (1 hunks)
  • src/Codebelt.Extensions.AspNetCore.Text.Yaml/Codebelt.Extensions.AspNetCore.Text.Yaml.csproj (1 hunks)
  • src/Codebelt.Extensions.YamlDotNet/Codebelt.Extensions.YamlDotNet.csproj (1 hunks)
  • test/Codebelt.Extensions.AspNetCore.Text.Yaml.Tests/Codebelt.Extensions.AspNetCore.Text.Yaml.Tests.csproj (1 hunks)
  • test/Codebelt.Extensions.YamlDotNet.Tests/Codebelt.Extensions.YamlDotNet.Tests.csproj (1 hunks)
  • test/Codebelt.Extensions.YamlDotNet.Tests/YamlSerializerTest.cs (0 hunks)
💤 Files with no reviewable changes (1)
  • test/Codebelt.Extensions.YamlDotNet.Tests/YamlSerializerTest.cs
✅ Files skipped from review due to trivial changes (4)
  • Directory.Packages.props
  • src/Codebelt.Extensions.YamlDotNet/Codebelt.Extensions.YamlDotNet.csproj
  • test/Codebelt.Extensions.AspNetCore.Text.Yaml.Tests/Codebelt.Extensions.AspNetCore.Text.Yaml.Tests.csproj
  • test/Codebelt.Extensions.YamlDotNet.Tests/Codebelt.Extensions.YamlDotNet.Tests.csproj
🧰 Additional context used
🪛 LanguageTool
README.md

[style] ~29-~29: The phrase ‘feel free to’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others and make it sound more professional.
Context: ...UTING.md) are welcome and appreciated. Feel free to submit issues, feature requests, or pul...

(FEEL_FREE_TO_STYLE_ME)

🔇 Additional comments (9)
src/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Text.Yaml.csproj (2)

14-14: Verify centralized package version configuration

The removal of the explicit version aligns with the move to centralized version management. Please ensure:

  1. The version is properly defined in Directory.Packages.props
  2. The version constraints are appropriate for .NET 9 compatibility
#!/bin/bash
# Check if the package version is defined in Directory.Packages.props
rg "Cuemon.AspNetCore.Mvc" "**/Directory.Packages.props"

# Check for any other references to this package that might still have explicit versions
rg -g "*.csproj" "Include=\"Cuemon.AspNetCore.Mvc\".*Version"

Line range hint 3-6: Verify compatibility with .NET 9 preview

Since .NET 9 is still in preview, please ensure:

  1. All dependencies are compatible with .NET 9
  2. Users are informed about potential preview-related issues
  3. Consider keeping .NET 8 as the primary target until .NET 9 is officially released
✅ Verification successful

Targeting .NET 9 preview is well-managed and appropriate

The codebase has proper infrastructure in place for .NET 9 targeting:

  • Centralized framework management in Directory.Build.props with appropriate conditionals
  • Non-test projects also target netstandard2.0 for broader compatibility
  • Test projects have OS-specific framework targeting
  • Package references are centrally managed without version specifications
  • Build configuration includes proper handling of CI, deterministic builds, and signing
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if any dependencies have compatibility issues with .NET 9
# Look for framework-specific conditional compilation
rg -g "*.cs" "#if NET9_0" 

# Check for any explicit .NET 9 dependencies or conditions in other project files
fd -e csproj -x grep -l "net9.0"

Length of output: 186


Script:

#!/bin/bash
# Check for .NET 9 targeting across all project files
fd -e csproj -x cat {} | grep -B2 -A2 "net9.0"

# Check package references and dependencies
fd -e csproj -x cat {} | grep -A5 "<PackageReference"

# Check for Directory.Build.props or similar central version management
fd "Directory.Build.props" -x cat {}

Length of output: 6581

src/Codebelt.Extensions.AspNetCore.Text.Yaml/Codebelt.Extensions.AspNetCore.Text.Yaml.csproj (2)

14-15: Verify centralized package version management setup

The removal of version specifications aligns with the move to centralized version management. However, we should verify that these package versions are properly defined in Directory.Packages.props.

✅ Verification successful

Centralized package version management is properly configured

The verification confirms that both packages are correctly defined in Directory.Packages.props:

  • Cuemon.AspNetCore version 9.0.0-rc.1
  • Cuemon.Extensions.DependencyInjection version 9.0.0-rc.1

No explicit versions were found in .csproj files, indicating successful migration to centralized version management.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify package version management setup

# Check if Directory.Packages.props exists and contains the package versions
if [ -f "Directory.Packages.props" ]; then
    echo "Checking Directory.Packages.props for package versions..."
    rg -A 1 'Cuemon\.(?:AspNetCore|Extensions\.DependencyInjection)' Directory.Packages.props
else
    echo "Directory.Packages.props not found!"
fi

# Check for any remaining explicit versions of these packages
rg -g '*.csproj' 'Include="Cuemon\.(?:AspNetCore|Extensions\.DependencyInjection)".*Version='

Length of output: 767


Line range hint 3-6: Verify the impact of dropping support for older .NET versions

The removal of support for older .NET versions (e.g., .NET 6) is a breaking change that should be clearly documented in release notes. Additionally, since .NET 9 is still in preview, ensure that all dependencies are compatible.

✅ Verification successful

Framework changes are properly documented and aligned across the solution

The framework changes are well-managed and documented:

  1. The CHANGELOG.md clearly documents this as part of v9.0.0 release, focusing on .NET 9 compatibility
  2. Directory.Build.props shows centralized version management with proper framework targeting:
    • Libraries: net9.0;net8.0;netstandard2.0
    • Test projects: net9.0;net8.0 (Linux) or net9.0;net8.0;net48 (Windows)
  3. All project files in the solution consistently target net9.0;net8.0 for ASP.NET Core specific packages

The changes are intentional, well-coordinated across the solution, and properly documented as part of the v9.0.0 release preparation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for .NET version references and release notes
# Check for any remaining .NET 6 references that might need updating
rg -g '*.csproj' 'net6.0'

# Check if release notes mention breaking change
rg -g 'PackageReleaseNotes.txt' -i 'breaking|.net 6|framework'

# Check if README mentions supported frameworks
cat README.md | grep -i 'framework\|prerequisites\|requirements'

Length of output: 304


Script:

#!/bin/bash
# Let's try a different approach to find relevant information

# Check all csproj files in the repository to understand framework targeting
fd -e csproj -x cat {}

# Look for any CHANGELOG or version history files
fd -i "changelog|release|version" -x cat {}

# Check Directory.Build.props for centralized version management
fd "Directory.Build.props" -x cat {}

Length of output: 14711

.nuget/Codebelt.Extensions.AspNetCore.Text.Yaml/PackageReleaseNotes.txt (2)

Line range hint 1-4: LGTM! Version number and framework support are correctly documented.

The version bump to 9.0.0 is appropriate given the breaking changes, and the supported frameworks (.NET 9 and .NET 8) are clearly stated.

🧰 Tools
🪛 LanguageTool

[style] ~5-~5: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...GED Dependencies to latest and greatest with respect to TFMs

  • REMOVED Support for TFM .NET 6 ...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


8-11: LGTM! New features are well documented.

The new features and changes are clearly documented with appropriate technical details:

  • New AddProblemDetailsConverter extension method
  • Enhanced AddHttpExceptionDescriptorConverter with Instance and TraceId parameters
  • Updated AddYamlExceptionResponseFormatter with ProblemDetails support
Directory.Build.props (2)

45-46: LGTM! Verify central package version management setup.

The removal of explicit versions from build-time tools aligns with the centralized version management approach. Both packages are correctly marked with PrivateAssets="all".

Let's verify the central package version configuration:

#!/bin/bash
# Description: Verify central package version management setup
# Expected: Directory.Packages.props exists and contains versions for these packages

# Check if Directory.Packages.props exists and contains versions
if [ -f "Directory.Packages.props" ]; then
    echo "Checking package versions in Directory.Packages.props:"
    cat Directory.Packages.props | grep -A 1 "Microsoft.SourceLink.GitHub\|MinVer"
else
    echo "Warning: Directory.Packages.props not found"
fi

72-84: Consider version constraints for critical test dependencies.

While centralizing package versions is good practice, test infrastructure might benefit from version constraints to ensure consistent test execution across environments. Consider adding version constraints in Directory.Packages.props for critical test dependencies.

Key packages to consider:

  • xunit and its runners (major version alignment)
  • Microsoft.NET.Test.Sdk (compatibility with target frameworks)
  • coverlet tools (version compatibility between msbuild and collector)

Let's verify the test package configurations:

CHANGELOG.md (1)

10-12: LGTM! Clear version header and introduction.

The version header follows semantic versioning, and the introduction clearly states the focus on .NET 9 compatibility.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~12-~12: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... introduced with .NET 9 preview releases so the final release is production ready t...

(COMMA_COMPOUND_SENTENCE_2)

Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.52%. Comparing base (261de45) to head (b76591d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #7   +/-   ##
=======================================
  Coverage   89.52%   89.52%           
=======================================
  Files          17       17           
  Lines         611      611           
  Branches       57       57           
=======================================
  Hits          547      547           
  Misses         64       64           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@gimlichael gimlichael merged commit d45ebc4 into main Nov 11, 2024
21 checks passed
@gimlichael gimlichael deleted the v9.0.0/prepare-for-launch branch November 11, 2024 21:24
@coderabbitai coderabbitai bot mentioned this pull request Jan 28, 2025
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.

1 participant