Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add OpenFeature.Extensions.Hosting package #181

Conversation

austindrenski
Copy link
Member

@austindrenski austindrenski commented Jan 16, 2024

Closes: #264


Related

TODO:

  • moar tests
    • evaluation context
      • IHostingEnvironment sample
    • hooks
    • providers:
      • none
      • one
      • multiple
  • xml docs
  • readme docs

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

Attention: Patch coverage is 65.78947% with 39 lines in your changes missing coverage. Please review.

Project coverage is 90.82%. Comparing base (11a0333) to head (bcf19c4).
Report is 11 commits behind head on main.

Current head bcf19c4 differs from pull request most recent head 9daa33f

Please upload reports for the commit 9daa33f to get more accurate results.

Files Patch % Lines
...Extensions.Hosting/OpenFeatureBuilderExtensions.cs 58.10% 29 Missing and 2 partials ⚠️
src/Shared/CallerArgumentExpressionAttribute.cs 0.00% 5 Missing ⚠️
src/Shared/Check.cs 66.66% 1 Missing and 1 partial ⚠️
...sions.Hosting/Internal/OpenFeatureHostedService.cs 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #181      +/-   ##
==========================================
- Coverage   95.40%   90.82%   -4.59%     
==========================================
  Files          27       29       +2     
  Lines        1111     1068      -43     
  Branches      120      113       -7     
==========================================
- Hits         1060      970      -90     
- Misses         34       70      +36     
- Partials       17       28      +11     

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

@austindrenski
Copy link
Member Author

austindrenski commented Jan 17, 2024

FYI couple of known test failures:

  1. DCO is intentionally broken by a temp commit containing a squash of fix: Fix ArgumentOutOfRangeException for empty hooks #187 and fix: More robust shutdown/cleanup/reset #188. Once those PRs are merged, I'll rebase and elide the temp commit.
  2. dotnet-format is being weird about OpenFeatureHostedService and I'm pretty sure its just a sign that we need to migrate from the dotnet-format tool, to the new dotnet format that's baked into recent SDKs.
  3. flaky unit tests: pretty sure these are all related to the async stuff, so hoping they'll be less common after chore(deps): update dependency flagsmith to v5.3.0 dotnet-sdk-contrib#184 lands, but in the meantime please just re-run the test if its failed at the time of your review.

@austindrenski austindrenski marked this pull request as ready for review January 17, 2024 16:03
@austindrenski austindrenski requested a review from a team as a code owner January 17, 2024 16:03
@austindrenski austindrenski force-pushed the add-open-feature-extensions-hosting branch from a9fd12e to 9821a5b Compare January 17, 2024 16:14
@austindrenski austindrenski force-pushed the add-open-feature-extensions-hosting branch from 9821a5b to 2eb7335 Compare January 17, 2024 18:50
@austindrenski austindrenski self-assigned this Jan 18, 2024
@austindrenski austindrenski force-pushed the add-open-feature-extensions-hosting branch 2 times, most recently from 828cebb to cf419bd Compare January 20, 2024 00:01
@austindrenski austindrenski force-pushed the add-open-feature-extensions-hosting branch 7 times, most recently from 7fb289d to a2cde68 Compare January 27, 2024 20:08
askpt added 10 commits May 14, 2024 17:50
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
# Conflicts:
#	Directory.Packages.props
#	README.md
… Fix unit tests.

Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
askpt added 10 commits July 24, 2024 08:37
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
# Conflicts:
#	Directory.Packages.props
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
@nbr28
Copy link

nbr28 commented Sep 24, 2024

Is this PR going to be merged? I was looking forward to this capabilities with version 2.0

@toddbaert
Copy link
Member

Is this PR going to be merged? I was looking forward to this capabilities with version 2.0

@nbr28 This is still being worked on, mostly by @askpt 🙏 . We are still planning to merge once it's completed.


<PropertyGroup>
<Nullable>enable</Nullable>
<TargetFrameworks>netstandard2.0;net6.0;net7.0;net8.0;net462</TargetFrameworks>

Choose a reason for hiding this comment

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

Should .NET 7 be included as TFM? .NET 7 is out of official support

/// Describes a <see cref="OpenFeatureBuilder"/> backed by an <see cref="IServiceCollection"/>.
/// </summary>
/// <param name="Services"><see cref="IServiceCollection"/></param>
public sealed record OpenFeatureBuilder(IServiceCollection Services);
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason to go for a record and the extension methods here? I am not too deep into .NET but I have not seen that pattern formerly.

@askpt askpt removed this from the 2.0 (breaking) milestone Oct 8, 2024
@askpt
Copy link
Member

askpt commented Oct 14, 2024

Superseded by #310

@askpt askpt closed this Oct 14, 2024
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.

Introduce OpenFeature.Extensions.Hosting package
9 participants