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: GO Feature Flag dotnet provider #24

Merged
merged 10 commits into from
Dec 1, 2022

Conversation

thomaspoignant
Copy link
Member

Description

This PR adds a new provider for GO Feature Flag.

Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
@thomaspoignant thomaspoignant marked this pull request as ready for review November 28, 2022 15:00
@toddbaert
Copy link
Member

toddbaert commented Nov 29, 2022

Thanks, as always, @thomaspoignant.

@kinyoklion @benjiro please don't feel obligated to review this, but I've added you if you're interested. I'll be taking a look at it myself as well.

@thomaspoignant thomaspoignant force-pushed the go-feature-flag-provider branch 2 times, most recently from 5ad4478 to 4fd777a Compare November 30, 2022 16:13
- Using better way to checks the options
- Using string interpolation
- Throw proper exception when we have parsing error.

Signed-off-by: Thomas Poignant <[email protected]>
@thomaspoignant thomaspoignant requested review from toddbaert and removed request for benjiro and kinyoklion November 30, 2022 16:17
@thomaspoignant
Copy link
Member Author

I don't know why GitHub has removed @benjiro and @kinyoklion from this one when I have re-request the approval from @toddbaert 😞

@toddbaert toddbaert requested review from benjiro, kinyoklion and beeme1mr and removed request for kinyoklion November 30, 2022 19:11
@toddbaert
Copy link
Member

toddbaert commented Nov 30, 2022

I think #24 (comment) is my only remaining concern. Unless you know for sure that there's no need to call .Dispose() on the Enumerator, I think we should, or we might have memory leaks or a lot of GC.

@thomaspoignant thomaspoignant requested review from toddbaert and removed request for benjiro and beeme1mr November 30, 2022 22:39
Signed-off-by: Thomas Poignant <[email protected]>
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

Thanks @thomaspoignant ! I've approved!

If you could add just one more thing before merge: a small readme!

Signed-off-by: Thomas Poignant <[email protected]>
@thomaspoignant
Copy link
Member Author

If you could add just one more thing before merge: a small readme!

@toddbaert I have added the README, I guess we are good to go.

@toddbaert toddbaert merged commit 964cf32 into open-feature:main Dec 1, 2022
This was referenced Dec 1, 2022
bacherfl pushed a commit to bacherfl/dotnet-sdk-contrib that referenced this pull request Jan 31, 2023
GO Feature Flag dotnet provider

Signed-off-by: Thomas Poignant <[email protected]>
vpetrusevici pushed a commit to vpetrusevici/open-feature-dotnet-sdk-contrib that referenced this pull request Oct 18, 2023
GO Feature Flag dotnet provider

Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Vladimir Petrusevici <[email protected]>
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.

2 participants