feat: .NET Standard 2.0 Support (AI refactor test) #104
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your project status has failed because the head coverage (24.49%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.
Additional details and impacted files@@ Coverage Diff @@
## main #104 +/- ##
==========================================
- Coverage 33.00% 24.49% -8.51%
==========================================
Files 128 138 +10
Lines 6387 8649 +2262
Branches 840 1735 +895
==========================================
+ Hits 2108 2119 +11
- Misses 4099 6180 +2081
- Partials 180 350 +170 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| [Fact] | ||
| public void TestHttpClientFunctionality() { | ||
| // Create an HTTP client (should work across all frameworks) | ||
| var client = new System.Net.Http.HttpClient(); |
Check warning
Code scanning / CodeQL
Missing Dispose call on local IDisposable Warning test
| var request = new System.Net.Http.HttpRequestMessage( | ||
| System.Net.Http.HttpMethod.Get, | ||
| "https://example.com"); |
Check warning
Code scanning / CodeQL
Missing Dispose call on local IDisposable Warning test
| catch (Exception e) { | ||
| foreach (var tupleKey in writes) { | ||
| writeResponses.Add(new ClientWriteSingleResponse { | ||
| TupleKey = tupleKey.ToTupleKey(), | ||
| Status = ClientWriteStatus.FAILURE, | ||
| Error = e, | ||
| }); | ||
| } | ||
| }); | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause Note
| catch (Exception e) { | ||
| foreach (var tupleKey in deletes) { | ||
| deleteResponses.Add(new ClientWriteSingleResponse { | ||
| TupleKey = tupleKey.ToTupleKey(), | ||
| Status = ClientWriteStatus.FAILURE, | ||
| Error = e, | ||
| }); | ||
| } | ||
| }); | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause Note
| catch (Exception e) { | ||
| responses.Add(new BatchCheckSingleResponse { Allowed = false, Request = request, Error = e }); | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause Note
|
@ryanpq - just b/c this PR has unrelated changes, would you mind approving and merging the sync PRs on the generator and here, rebasing your generator PR and then refreshing this one? That should clear up all the unrelated changes |
|
@Hawxy - We've gotten a few requests to support .NET Standard 2.0 which should be compatible with .NET Core 6 through 8. I'd love to get your thoughts on this, especially with avoiding any breaking to https://github.com/Hawxy/Fga.Net. Will us moving from .NET Core 6 to .NET Standard 2.0 disrupt your work? Anything we need to watch out for from a breaking change perspective for other users? Do you see any issues in the way @ryanpq implemented it here? |
|
This won't be a breaking change, however it would be good to multi-target an LTS version (net8.0) as an additional target. |
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="7.0.0" /> | ||
| <PackageReference Include="System.ComponentModel.Annotations" Version="5.0.0" /> |
src/OpenFga.Sdk/OpenFga.Sdk.csproj
Outdated
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="7.0.0" /> | ||
| <PackageReference Include="System.ComponentModel.Annotations" Version="5.0.0" /> | ||
| <PackageReference Include="System.Net.Http.Json" Version="7.0.1" /> |
There was a problem hiding this comment.
These other dependencies should be kept in line with .NET LTS versions as 7.x.x is end of life.
| // | ||
|
|
||
|
|
||
| namespace OpenFga.Sdk.Telemetry { |
There was a problem hiding this comment.
OTEL support is completely broken in this PR. System.Diagnostics.DiagnosticSource probably needs to be installed and this code deleted.
| <GeneratePackageOnBuild>true</GeneratePackageOnBuild> | ||
| <!-- Disable implicit usings for .NET Standard 2.0 --> | ||
| <ImplicitUsings>disable</ImplicitUsings> | ||
| <DefineConstants>NETSTANDARD2_0</DefineConstants> |
There was a problem hiding this comment.
This constant already exists in the framework
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the SDK to support .NET Standard 2.0 while adding compatibility tests for .NET Core 3.1 and .NET Framework 4.8. Key changes include new tests for .NET Standard support, updates to examples and documentation (e.g. BatchCheck, retry count adjustments), and minor solution file and Makefile updates.
Reviewed Changes
Copilot reviewed 155 out of 155 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/OpenFga.Sdk.Test.NetCore31/NetCore31CompatibilityTests.cs | Added tests to verify .NET Core 3.1 support |
| src/OpenFga.Sdk.Test.Framework/FrameworkCompatibilityTests.cs | Added tests for .NET Framework 4.8 compatibility |
| example/Example1/Example1.csproj | Downgraded package reference version and added .NET Standard instructions |
| example/Example1/Example1.cs | Updated example code to use new object IDs and remove unused booleans |
| docs/* | Updated API documentation to include new endpoints and updated response codes |
| OpenFga.Sdk.sln, Makefile, CHANGELOG.md | Various project configuration and changelog updates |
Comments suppressed due to low confidence (2)
example/Example1/Example1.csproj:13
- The package reference version was changed from 0.5.1 to 0.2.5. Please confirm that downgrading to 0.2.5 is intentional and consistent with the intended .NET Standard 2.0 support.
<PackageReference Include="OpenFga.Sdk" Version="0.2.5"><PrivateAssets>all</PrivateAssets></PackageReference>
README.md:100
- Ensure that the retry count documented here (3 retries) matches the current client implementation, as it was reduced from 15 retries.
> The `OpenFga.SdkClient` will by default retry API requests up to 3 times on 429 and 5xx errors.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Allowed == other.Allowed && | ||
| (Request?.Equals(other.Request) ?? other.Request == null) && | ||
| ((Error == null && other.Error == null) || | ||
| (Error != null && other.Error != null && Error.Message == other.Error.Message)); |
Check notice
Code scanning / CodeQL
Complex condition Note
4 participants
Description
DO NOT MERGE
This is an initial test of utilizing AI tools to refactor the dotNET SDK to support dotNET Standard 2.0. This branch is intended as a test and reference only.
References
openfga/sdk-generator#536
Closes: #106
Review Checklist
main