Skip to content

feat: implement basic functionality of OFREP provider #88

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
Merged
merged 8 commits into from
Oct 2, 2024
Merged

feat: implement basic functionality of OFREP provider #88

merged 8 commits into from
Oct 2, 2024

Conversation

@federicobond
Copy link
Member

@federicobond federicobond commented May 7, 2024

No description provided.

@codecov
Copy link

codecov bot commented May 7, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.45%. Comparing base (d8e10c7) to head (b15f651).
Report is 37 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #88      +/-   ##
==========================================
+ Coverage   90.55%   92.45%   +1.90%     
==========================================
  Files           8       12       +4     
  Lines         180      464     +284     
==========================================
+ Hits          163      429     +266     
- Misses         17       35      +18
Flag Coverage Δ
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@federicobond federicobond force-pushed the feat/ofrep-provider-impl branch from 1ca6a60 to a9c96f2 Compare May 7, 2024 03:07
@federicobond federicobond marked this pull request as ready for review May 7, 2024 06:01
@federicobond federicobond requested a review from a team as a code owner May 7, 2024 06:01
@federicobond federicobond force-pushed the feat/ofrep-provider-impl branch from 9b41a80 to 535fd27 Compare May 8, 2024 03:15
@federicobond federicobond linked an issue May 8, 2024 that may be closed by this pull request
Implement OFREP Provider #79
Closed
lukas-reining
lukas-reining reviewed May 10, 2024
View reviewed changes
Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

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

Looks great to me and feels the same as for JS.
I left one question regarding validation of evaluation results.

beeme1mr
beeme1mr approved these changes May 10, 2024
View reviewed changes
@Kavindu-Dodan
Copy link

Kavindu-Dodan commented May 10, 2024

Good start 🙌

My main concern is this

@thomaspoignant thomaspoignant self-requested a review May 13, 2024 07:37
gruebel
gruebel reviewed May 13, 2024
View reviewed changes
Copy link
Member

@gruebel gruebel left a comment

Choose a reason for hiding this comment

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

nice job technically, just added some small suggestions

@federicobond federicobond force-pushed the feat/ofrep-provider-impl branch from b9eec89 to b7b5e27 Compare May 21, 2024 09:08
@lukas-reining
Copy link
Member

lukas-reining commented Aug 22, 2024

Hey @open-feature/sdk-python-maintainers, what is missing from your side, especially @federicobond to get this released?
It would be great for us to have OFREP in Python, so if I can help to get it through, I would be happy to help.

@gaby gaby mentioned this pull request Aug 22, 2024
Closed
@beeme1mr
Copy link
Member

beeme1mr commented Oct 2, 2024

@lukas-reining, I think it's worth getting this out there. I'll merge when the CI passes. Thanks for kicking this off, @federicobond, and sorry for the delay.

@beeme1mr beeme1mr merged commit cb42cc0 into open-feature:main Oct 2, 2024
17 checks passed
@github-actions github-actions bot mentioned this pull request Oct 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gruebel gruebel gruebel left review comments

@lukas-reining lukas-reining lukas-reining left review comments

@beeme1mr beeme1mr beeme1mr approved these changes

@toddbaert toddbaert Awaiting requested review from toddbaert

@thomaspoignant thomaspoignant Awaiting requested review from thomaspoignant

+1 more reviewer

@Kavindu-Dodan Kavindu-Dodan Kavindu-Dodan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement OFREP Provider

5 participants

@federicobond @Kavindu-Dodan @lukas-reining @beeme1mr @gruebel