Conversation
Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
…e Objects Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
…nstead Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Error is no longer a record struct, but implements IEquatable<T> directly. It offers overloads to exclude Metadata comparison. Errors does the same thing now, it supports leaving out metadata comparison. Signed-off-by: Kenny Pflug <[email protected]>
…HasOptionalMetadata<T> This involves Correlation ID and Source. This commit also includes a refactoring of the metadata integration in Result<T> and Result by introducing the IHasOptionalMetadata<T> interface. Metadata related methods are now mostly extension methods. Signed-off-by: Kenny Pflug <[email protected]>
Signed-off-by: Kenny Pflug <[email protected]>
Further implementation notes:Tracing support via metadata extensions (architectural decision)Original plan: Add Implemented approach: Keep Rationale for deviation:
Implementation details:
Trade-offs accepted:
Serialization implications:
|
Signed-off-by: Kenny Pflug <[email protected]>
This is actually related to C# 14 Extension Blocks. I had to revert back to Extension Methods to fix this. See MetadataExtensions.cs for details. Signed-off-by: Kenny Pflug <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR optimizes Result<T> and Errors for performance by implementing Small Buffer Optimization (SBO) for error storage, replacing ImmutableArray<Error> with a struct-based Errors type that exposes ReadOnlyMemory<Error>, and adding TryGetValue with proper nullability. The Error struct now includes Category, Source, CorrelationId fields (via metadata extensions), and uses required properties. The MetadataObjectExtensions.MergeIfNeeded method reduces redundant metadata merges.
Changes:
- Replaced
ImmutableArray<Error>withErrorsstruct using SBO andReadOnlyMemory<Error> - Added
Result<T>.TryGetValueand changedIsSuccess/IsFailuretoIsValid - Enhanced
ErrorwithCategoryenum and validation, removed record struct pattern - Implemented metadata optimization via
MergeIfNeededandIHasOptionalMetadata<T>interface
Reviewed changes
Copilot reviewed 79 out of 81 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Light.Results/Error.cs | Converted from record struct to regular struct with required Message property, added Category enum, implemented custom equality/hashing |
| src/Light.Results/ErrorCategory.cs | New enum mapping HTTP status codes to error categories |
| src/Light.Results/Errors.cs | Complete rewrite with SBO, ReadOnlyMemory storage, custom enumerator, and optimized equality checking |
| src/Light.Results/Result.cs | Changed IsSuccess/IsFailure to IsValid, added TryGetValue, replaced ErrorList with Errors property, added equality operators |
| src/Light.Results/Metadata/*.cs | Added IHasOptionalMetadata interface, MergeIfNeeded optimization, value-based equality for MetadataValue/Object/Array |
| src/Light.Results/MetadataExtensions/Tracing.cs | New tracing extensions for Source and CorrelationId via metadata |
| tests/* | Comprehensive test coverage for all new functionality |
Comments suppressed due to low confidence (1)
src/Light.Results/ErrorCategory.cs:1
- Typo in test method name: 'UnavailableForLegaReasons' should be 'UnavailableForLegalReasons' (missing 'l' in 'Legal').
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Kenny Pflug <[email protected]>
Errorsnow exposes SBO storage viaReadOnlyMemory<Error>and an allocation-free enumerator.Result<T>.Errorsreplaces the oldImmutableArraycopy,TryGetValueis implemented with the proper nullability attributes, and the non-generic wrapper delegates accordingly.Errornow carriesSource,CorrelationId, andCategory, with helper factories/tests validating defaults and fluent APIs.MetadataObjectExtensions.MergeIfNeededexists and is used, reducing redundant merges