app/featureset: implement feature flags #446
Conversation
Codecov Report
@@ Coverage Diff @@
## main #446 +/- ##
==========================================
+ Coverage 54.87% 55.01% +0.14%
==========================================
Files 66 70 +4
Lines 6141 6218 +77
==========================================
+ Hits 3370 3421 +51
- Misses 2309 2329 +20
- Partials 462 468 +6
Continue to review full report at Codecov.
|
|
|
||
| const ( | ||
| // QBFTConsensus introduces qbft consensus, see https://github.com/ObolNetwork/charon/issues/445. | ||
| QBFTConsensus Feature = "qbft_consensus" |
There was a problem hiding this comment.
i think we can also have proposer feature here
There was a problem hiding this comment.
Can add that in next PR
| // state defines the current rollout status of each feature. | ||
| state = map[Feature]status{ | ||
| QBFTConsensus: statusAlpha, | ||
| // Add all features and there status here. |
There was a problem hiding this comment.
here too proposer feature with statusAlpha
| } | ||
|
|
||
| // Init initialises the global feature set state. | ||
| func Init(ctx context.Context, config Config) error { |
There was a problem hiding this comment.
so basically minStatus is very important and enabled and disabled are not that important it seems
There was a problem hiding this comment.
yup, minStatus enables/disables different sets of features. While Enable/Disable is to override individual features
| @@ -73,6 +74,11 @@ func TestCmdFlags(t *testing.T) { | |||
| Denylist: "", | |||
| DBPath: "", | |||
| }, | |||
| Feature: featureset.Config{ | |||
There was a problem hiding this comment.
nit: add cmd test including enabled and disabled features too so that reader of this code would get more clarity how to use these flags.
Because from the name of these flags it seems that they are boolean flags (like enabled=true/false).
There was a problem hiding this comment.
I'll see what I can do
corverroos
added
the
merge when ready
Implement
featuresetpackage as per design proposal.category: feature
ticket: #381