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.

Sign up for GitHub

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

Jump to bottom

feat: Go feature flag provider #106

Merged
merged 10 commits into from
Sep 26, 2022
Merged

feat: Go feature flag provider #106

merged 10 commits into from
Sep 26, 2022

Conversation

thomaspoignant
Copy link
Member

First version of the provider for GO Feature Flag.

@thomaspoignant thomaspoignant added the enhancement New feature or request label Sep 22, 2022
@thomaspoignant thomaspoignant self-assigned this Sep 22, 2022
@thomaspoignant thomaspoignant changed the title Go feature flag provider feat: Go feature flag provider Sep 22, 2022
@thomaspoignant
Copy link
Member Author

The stage Configure GPG Key in the CI is failing.
I guess this is because this PR is open from a fork, maybe we should have an issue for that.

@toddbaert
Copy link
Member

The stage Configure GPG Key in the CI is failing. I guess this is because this PR is open from a fork, maybe we should have an issue for that.

I was just about to make a comment about this. I will fix this in another PR and you can rebase on it. It will take only a moment.

@toddbaert
Copy link
Member

toddbaert commented Sep 22, 2022 •
edited
Loading

#107 after this is merged you can rebase if you want. Sorry about that!

@toddbaert toddbaert self-requested a review September 22, 2022 20:57
toddbaert
toddbaert requested changes Sep 22, 2022
View reviewed changes
Copy link
Member

@toddbaert toddbaert left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I left a few minor comments for you to think about, but this all looks pretty good to me!

You will need to add this to the main pom.xml's modules element though, it's not currently actually building:

[INFO] Reactor Summary:
[INFO] 
[INFO] java-sdk-contrib 0.0.2 ............................. SUCCESS [  4.806 s]
[INFO] open-telemetry-hook 0.2.8 .......................... SUCCESS [ 14.724 s]
[INFO] flagd 0.3.2 ........................................ SUCCESS [ 25.281 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS

java-sdk-contrib/pom.xml

Lines 29 to 32 in bc8e3ce

<modules>
<module>hooks/open-telemetry</module>
<module>providers/flagd</module>
</modules>

@thomaspoignant
Fix feedback on the PR
7d9f536 
Signed-off-by: Thomas Poignant <[email protected]>
@thomaspoignant
Add connection pool for OkHttp
304c748 
Signed-off-by: Thomas Poignant <[email protected]>
@toddbaert toddbaert self-requested a review September 23, 2022 14:09
toddbaert
toddbaert approved these changes Sep 23, 2022
View reviewed changes
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.

Very nice!

@toddbaert
Copy link
Member

@thomaspoignant dependency is available now! Everything looks good to me except a few lint issues.

@thomaspoignant thomaspoignant force-pushed the go-feature-flag-provider branch from b5d4171 to 50c5225 Compare September 23, 2022 15:26
@thomaspoignant
Fix parent version
949f971 
Signed-off-by: Thomas Poignant <[email protected]>
@thomaspoignant thomaspoignant force-pushed the go-feature-flag-provider branch 2 times, most recently from 13d3ef1 to b592aa0 Compare September 23, 2022 15:54
@thomaspoignant
Linting issue
6db011c 
Signed-off-by: Thomas Poignant <[email protected]>
@thomaspoignant thomaspoignant force-pushed the go-feature-flag-provider branch from b592aa0 to 6db011c Compare September 23, 2022 15:56
@thomaspoignant
Fix findbugs issue
034878b 
Signed-off-by: Thomas Poignant <[email protected]>
@thomaspoignant
Copy link
Member Author

thomaspoignant commented Sep 24, 2022 •
edited
Loading

@toddbaert I've finally finished making lombok work with findbugs.
The actual configuration does not really like it 😉

Can you have a last check? It is ready to merge on my side.

@thomaspoignant thomaspoignant mentioned this pull request Sep 24, 2022
Closed
@toddbaert toddbaert merged commit f793722 into open-feature:main Sep 26, 2022
This was referenced Sep 26, 2022
Closed
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beeme1mr beeme1mr beeme1mr approved these changes

@toddbaert toddbaert toddbaert approved these changes

@justinabrahms justinabrahms Awaiting requested review from justinabrahms

Assignees

@thomaspoignant thomaspoignant

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thomaspoignant @toddbaert @beeme1mr