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 Statsig provider #641

Merged
merged 12 commits into from
Feb 2, 2024

Conversation

liran2000
Copy link
Member

@liran2000 liran2000 commented Jan 18, 2024

Readme describes the provider.

@MovieStoreGuy @jasonwzm @xinlili-statsig @toddbaert you are welcome to review.

@liran2000 liran2000 requested a review from a team as a code owner January 18, 2024 09:18
toddbaert
toddbaert previously approved these changes Jan 31, 2024
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.

LGTM @liran2000 , thanks again.

@xinlili-statsig are you interested in being added to the component_owners for this component? It will mean you are added to PRs that impact it. Note you will have to join the OpenFeature org in that case.

@liran2000 please add yourself to https://github.com/open-feature/java-sdk-contrib/blob/main/.github/component_owners.yml for this at a minimum.

Also, you may be interested in this issue since you've authored quite a few providers.

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.

Hey @liran2000 I led you wrong, I said to use InvalidContextError but we have TargetingKeyMissingError, so I recommend that... so I made that change. Sorry!

@toddbaert
Copy link
Member

@Kavindu-Dodan when you have a moment, please review and feel free to merge if it looks OK.

Copy link

@xinlili-statsig xinlili-statsig left a comment

Choose a reason for hiding this comment

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

Overall lgtm!

Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
liran2000 and others added 3 commits February 2, 2024 14:39
Signed-off-by: liran2000 <liran2000@gmail.com>
…ers/statsig/ContextTransformer.java

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
…ers/statsig/ContextTransformer.java

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@Kavindu-Dodan
Copy link
Collaborator

Merging as this is approved by @xinlili-statsig. Also, looks good in my view and there's good test coverage.

@Kavindu-Dodan Kavindu-Dodan merged commit f814696 into open-feature:main Feb 2, 2024
4 checks passed
@liran2000 liran2000 deleted the feature/statsig branch February 3, 2024 07:05
DBlanchard88 pushed a commit to DBlanchard88/java-sdk-contrib that referenced this pull request Apr 29, 2024
…ure#641)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

5 participants