-
Notifications
You must be signed in to change notification settings - Fork 3
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.
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
feat: add featureFlags param #26
Conversation
34fecc0
to
a894df1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #26 +/- ##
==========================================
+ Coverage 63.21% 63.24% +0.02%
==========================================
Files 28 28
Lines 6206 6211 +5
==========================================
+ Hits 3923 3928 +5
Misses 2115 2115
Partials 168 168 ☔ View full report in Codecov by Sentry. |
return p.RunWithFeatureFlags(ctx, vars, store, nil) | ||
} | ||
|
||
func (p ParseResult) RunWithFeatureFlags( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we add a new publicly exposed function which has a featureFlags map[string]struct{}
param
We have a map with untyped keys instead of a struct so that removing feature flags is not a breaking change
a894df1
to
778896f
Compare
@@ -175,6 +175,7 @@ func RunProgram( | |||
program parser.Program, | |||
vars map[string]string, | |||
store Store, | |||
featureFlags map[string]struct{}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we pass the feature flags here directly (without further design patterns e.g. dependency injection). When a fflag will be available, we will parse it as a boolean field in the programState
struct
this way
- we can still control presence/absence of fflags in tests
- the runtime will have access to passed feature flags in each moment
this is noop PR, but it involves changing the publicly exposed function, which we'll want to design in a way that will be backwards compatible in the future