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]>
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]>
Signed-off-by: Kenny Pflug <[email protected]>
There was a problem hiding this comment.
Pull request overview
This pull request prepares the Light.Results library for its initial 0.1.0 release on NuGet. The library implements the Result Pattern with serialization support for HTTP and CloudEvents, including ASP.NET Core integrations.
Changes:
- Added comprehensive README.md with documentation, examples, and configuration guides
- Created GitHub Actions workflow for automated NuGet package publishing on release
- Added package metadata (descriptions, release notes, repository URLs, and licensing) to all project files
- Added Microsoft.SourceLink.GitHub for source debugging support
- Included package icon and public signing key
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Comprehensive documentation with quickstart examples, usage patterns, and configuration options |
| .github/workflows/release-on-nuget.yml | Automated workflow for building, signing, and publishing NuGet packages |
| Directory.Build.props | Added version 0.1.0 for the release |
| src/Directory.Build.props | Added NuGet package metadata including repository URLs, licensing, icon, and SourceLink configuration |
| Directory.Packages.props | Added Microsoft.SourceLink.GitHub package version and reordered coverlet.collector |
| src/Light.Results/Light.Results.csproj | Added package description and release notes for core library |
| src/Light.Results.AspNetCore.Shared/Light.Results.AspNetCore.Shared.csproj | Added package description and release notes for shared ASP.NET Core functionality |
| src/Light.Results.AspNetCore.Mvc/Light.Results.AspNetCore.Mvc.csproj | Added package description and release notes for MVC integration |
| src/Light.Results.AspNetCore.MinimalApis/Light.Results.AspNetCore.MinimalApis.csproj | Added package description and release notes for Minimal APIs integration |
| src/Light.Results/Http/Writing/MetadataSerializationMode.cs | Removed placeholder file (type moved to SharedJsonSerialization namespace) |
| Light.Results.Public.snk | Added public key file for assembly signing |
| images/light-logo.png | Added package icon |
| Light.Results.slnx | Updated solution to include new workflow and README files |
| Various packages.lock.json files | Updated with Microsoft.SourceLink.GitHub dependencies and version reference changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --------------------------------- | ||
|
|
||
| - Initial release 🚀 | ||
| - LightActionResult and LightActionResult<T> and corresponding extension methods to turn result instances into HTTP success responses or RFC 9457 (and RFC 7807) compatbile Problem Details responses. |
There was a problem hiding this comment.
Spelling error: "compatbile" should be "compatible".
| - LightActionResult and LightActionResult<T> and corresponding extension methods to turn result instances into HTTP success responses or RFC 9457 (and RFC 7807) compatbile Problem Details responses. | |
| - LightActionResult and LightActionResult<T> and corresponding extension methods to turn result instances into HTTP success responses or RFC 9457 (and RFC 7807) compatible Problem Details responses. |
| --------------------------------- | ||
|
|
||
| - Initial release 🚀 | ||
| - LightResult and LightResult<T> and corresponding extension methods to turn result instances into HTTP success responses or RFC 9457 (and RFC 7807) compatbile Problem Details responses. |
There was a problem hiding this comment.
Spelling error: "compatbile" should be "compatible".
| - LightResult and LightResult<T> and corresponding extension methods to turn result instances into HTTP success responses or RFC 9457 (and RFC 7807) compatbile Problem Details responses. | |
| - LightResult and LightResult<T> and corresponding extension methods to turn result instances into HTTP success responses or RFC 9457 (and RFC 7807) compatible Problem Details responses. |
| <EmbedUntrackedSources>true</EmbedUntrackedSources> | ||
| <PackageTags>streaming;memory-management;form-file</PackageTags> | ||
| <PackageIcon>light-logo.png</PackageIcon> | ||
| <PackageReadmeFile>readme.md</PackageReadmeFile> |
There was a problem hiding this comment.
The PackageReadmeFile references "readme.md" but the actual file is named "README.md" (uppercase). This case mismatch may cause issues on case-sensitive file systems. Update to "README.md" to match the actual filename.
| <PackageReadmeFile>readme.md</PackageReadmeFile> | |
| <PackageReadmeFile>README.md</PackageReadmeFile> |
| - name: Push NuGet packages | ||
| env: | ||
| NUGET_API_KEY: ${{ secrets.NUGET_API_KEY }} | ||
| run: dotnet nuget push "./src/**/*.nupkg" --api-key $NUGET_API_KEY --source https://api.nuget.org/v3/index.json |
There was a problem hiding this comment.
Consider adding the --skip-duplicate flag to the dotnet nuget push command. Without it, the workflow will fail if a package with the same version already exists on NuGet, which could happen during re-runs or if a release is republished.
| run: dotnet nuget push "./src/**/*.nupkg" --api-key $NUGET_API_KEY --source https://api.nuget.org/v3/index.json | |
| run: dotnet nuget push "./src/**/*.nupkg" --api-key $NUGET_API_KEY --source https://api.nuget.org/v3/index.json --skip-duplicate |
|
|
||
| - Initial release 🚀 | ||
| - Result enrichment with ASP.NET Core's HttpContext | ||
| - Shared logic for Minimal API's and MVC's composition root |
There was a problem hiding this comment.
Grammar issue: "Minimal API's" should be "Minimal APIs" (plural form, not possessive). Similarly, "MVC's" should be "MVC's" or simply "MVC".
| - Shared logic for Minimal API's and MVC's composition root | |
| - Shared logic for Minimal APIs and MVC's composition root |
| <IncludeSymbols>true</IncludeSymbols> | ||
| <SymbolPackageFormat>snupkg</SymbolPackageFormat> | ||
| <EmbedUntrackedSources>true</EmbedUntrackedSources> | ||
| <PackageTags>streaming;memory-management;form-file</PackageTags> |
There was a problem hiding this comment.
The PackageTags value "streaming;memory-management;form-file" appears to be incorrect for a Result Pattern library. These tags suggest file upload/streaming functionality, which doesn't match the library's purpose. Consider using tags like "result-pattern;error-handling;functional-programming;aspnetcore;cloudevents" instead.
| <PackageTags>streaming;memory-management;form-file</PackageTags> | |
| <PackageTags>result-pattern;error-handling;functional-programming;aspnetcore;cloudevents</PackageTags> |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
… push flag Co-authored-by: feO2x <[email protected]>
Fix review feedback: spelling, package tags, readme casing, NuGet push flag
Signed-off-by: Kenny Pflug <[email protected]>
Closes #19