open-feature / ruby-sdk

Ruby implementation of the OpenFeature SDK
https://openfeature.dev
Apache License 2.0
21 stars 7 forks source link

feat: add integer and float specific resolver methods #124

Closed mschoenlaub closed 4 months ago

mschoenlaub commented 4 months ago

This PR

Related Issues

Fixes #121

Notes

It's somewhat weird that the client's tests use an actual NoopProvider instead of a mock. That kind of couples these changes more than i'd like to. But that's just how it is for now :) I'm also wondering, if we wouldn't actually need a slightly more elaborated solution where the client falls back to the fetch_number method on the rovider, if it doesn't have a fetch_integer/fetch_float. The reason is, that currently the client blindly delegates these calls, and there might be providers out in the wild.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.48%. Comparing base (a6c789d) to head (af3cdb8).

:exclamation: Current head af3cdb8 differs from pull request most recent head 07c0341. Consider uploading reports for the commit 07c0341 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #124 +/- ## ========================================== + Coverage 99.46% 99.48% +0.02% ========================================== Files 15 15 Lines 187 195 +8 ========================================== + Hits 186 194 +8 Misses 1 1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mschoenlaub commented 4 months ago

@maxveldink , I definitely agree with your interpretation of the spec. It would also make a bunch of stuff easier, like fallback for optional methods and not having the same type checking code in every provider.

maxveldink commented 4 months ago

Cool, I've added #126 to track that work (hopefully I can get to it soon). I think I'll go ahead and merge this in and cut a new release so we can get this behavior released.

toddbaert commented 4 months ago

I was late to this review, but it all looks right to me from a spec perspective. Nice job and thanks @mschoenlaub and @maxveldink