-
Notifications
You must be signed in to change notification settings - Fork 23
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 Flagsmith provider #89
Conversation
Hey @dabeeeenster, could someone from the Flagsmith team please take a look when you have a moment? |
test/OpenFeature.Contrib.Hooks.Otel.Test/OpenFeature.Contrib.Hooks.Otel.Test.csproj
Outdated
Show resolved
Hide resolved
src/OpenFeature.Contrib.Providers.Flagsmith/FlagsmithProvider.cs
Outdated
Show resolved
Hide resolved
src/OpenFeature.Contrib.Providers.Flagsmith/FlagsmithProvider.cs
Outdated
Show resolved
Hide resolved
src/OpenFeature.Contrib.Providers.Flagsmith/FlagsmithProvider.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this addition! I made a few comments, but this looks like a great addition!
I'm marking as "request changes" for now, primarily because I'd like an approval from a representative from Flagsmith first.
Once we have identified a Flagsmith representative, I will add you and them to https://github.com/open-feature/dotnet-sdk-contrib/blob/main/.github/component_owners.yml as part of this PR.
ba2f401
to
74569a9
Compare
Co-authored-by: Matthew Elwell <[email protected]> Signed-off-by: vpetrusevici <[email protected]> Signed-off-by: Vladimir Petrusevici <[email protected]>
Signed-off-by: Vladimir Petrusevici <[email protected]>
Signed-off-by: Vladimir Petrusevici <[email protected]>
Signed-off-by: Vladimir Petrusevici <[email protected]>
Hi @beeme1mr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from a Flagsmith perspective.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just had one question about the icon, but the rest looks good. @toddbaert, did you have any other concerns?
Before we merge, could you lease update the component_owners.yml
to include @matthewelwell, novakzaballa, and yourself?
src/OpenFeature.Contrib.Providers.Flagsmith/openfeature-icon.png
Outdated
Show resolved
Hide resolved
Signed-off-by: Vladimir Petrusevici <[email protected]>
done |
Could you please also add a readme under |
Signed-off-by: Vladimir Petrusevici <[email protected]>
I just copied readme form flagd provider and adapted to Flagsmith provider. |
Co-authored-by: Michael Beemer <[email protected]> Signed-off-by: vpetrusevici <[email protected]>
Co-authored-by: Michael Beemer <[email protected]> Signed-off-by: vpetrusevici <[email protected]>
Co-authored-by: Michael Beemer <[email protected]> Signed-off-by: vpetrusevici <[email protected]>
@toddbaert can we complete the PR? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vpetrusevici very nice! I like the delegate implementation. This is a great addition!
I'm approving but you have some minor formatting issues (you should be able to test your fixes with dotnet-format --folder --check
). I'll keep an eye on the PR and re-run the checks / merge as soon as they pass.
Signed-off-by: Vladimir Petrusevici <[email protected]>
I added |
This PR
Adds a library project for Flagsmith provider with implementation and tests
Related Issues
#90
Flagsmith/flagsmith-dotnet-client#78
Notes
We are going to use OpenFeature and Flagsmith in our projects. Unfortunately dotnet-sdk-contrib doesn't have Flagsmith provider, so i implemented it.
Follow-up Tasks
How to test
Just create FlagsmithProvider using FlagsmithClient or FlagsmithConfiguration according to https://github.com/Flagsmith/flagsmith-dotnet-client/tree/main/Example