open-feature / python-sdk

Python SDK for OpenFeature
https://openfeature.dev
Apache License 2.0
48 stars 16 forks source link

fix: Allow string values for `FlagEvaluationDetails.reason` and `FlagResolutionDetails.reason` #264

Closed keelerm84 closed 7 months ago

keelerm84 commented 7 months ago

This PR

Expands the types allowed for reason on FlagEvaluationDetails and FlagResolutionDetails to include both Reason and str types.

Related Issues

Fixes #262

Notes

Related to spec issue: https://github.com/open-feature/spec/issues/236

codecov[bot] commented 7 months ago

Codecov Report

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

Comparison is base (f1b0839) 93.89% compared to head (fd30717) 93.89%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #264 +/- ## ======================================= Coverage 93.89% 93.89% ======================================= Files 16 16 Lines 442 442 ======================================= Hits 415 415 Misses 27 27 ``` | [Flag](https://app.codecov.io/gh/open-feature/python-sdk/pull/264/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/open-feature/python-sdk/pull/264/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | `93.89% <100.00%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#carryforward-flags-in-the-pull-request-comment) to find out more.

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

keelerm84 commented 7 months ago

Hi @keelerm84! Thank you for your contribution. Change LGTM, but I am lost on what the test is supposed to assert.

If it's just a type check then it should be already covered by mypy. Would any of the tests break if you reverted the type change?

That's a good point. I've removed the useless test and will rely on mypy as suggested.