Skip to content

refactor: make Reason enum values instances of str - #142

Merged
beeme1mr merged 1 commit into
open-feature:mainfrom
federicobond:strenum
Jul 12, 2023
Merged

refactor: make Reason enum values instances of str#142
beeme1mr merged 1 commit into
open-feature:mainfrom
federicobond:strenum

Conversation

@federicobond

Copy link
Copy Markdown
Member

This PR

Is a refinement on #126. In particular, it makes Reason values also instances of str, and thus directly comparable without having to access the .value property, and without having to define custom getters and setters in dataclasses.

The motivation for this was to fix a no-redef issue reported by mypy in the FlagEvaluationDetails definition:

open_feature/flag_evaluation/flag_evaluation_details.py:19: error: Name "reason" already defined on line 15 [no-redef]

StrEnum is available since Python 3.11 so it requires a minimal backport for earlier Python versions.

@federicobond
federicobondforce-pushed the strenum branch 3 times, most recently from bf8ebec to 5be2ae4CompareJuly 11, 2023 14:09
Signed-off-by: Federico Bond <federicobond@gmail.com>
@codecov

codecovBot commented Jul 11, 2023

Copy link
Copy Markdown

Codecov Report

Merging #142 (1e9e37f) into main (695da28) will not change coverage.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #142 +/- ##
=======================================
Coverage 93.26% 93.26% =======================================
Files 21 22 +1 Lines 386 386 =======================================
Hits 360 360 Misses 26 26 
FlagCoverage Δ
unittests93.26% <100.00%> (ø)

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

Impacted FilesCoverage Δ
...feature/flag_evaluation/flag_evaluation_details.py100.00% <ø> (ø)
open_feature/_backports/strenum.py100.00% <100.00%> (ø)
open_feature/flag_evaluation/reason.py100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@hlipsighlipsig left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't have any concerns here. This is a good change.

@beeme1mr
beeme1mr merged commit 633deff into open-feature:mainJul 12, 2023
kikihakiem pushed a commit to ResalApps/openfeature-python-sdk that referenced this pull request Aug 28, 2025
Sign up for freeto 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.

3 participants

@federicobond@beeme1mr@hlipsig