-
Notifications
You must be signed in to change notification settings - Fork 19
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 datafile accessor #290
Conversation
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.
please address.
@@ -48,6 +49,11 @@ type DatafileProjectConfig struct { | |||
botFiltering bool | |||
} | |||
|
|||
// GetDatafile returns a JSON string representation of the environment's datafile |
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.
returns a string representation, datafile is itself in JSON, don't need to add that.
pkg/config/optimizely_config.go
Outdated
datafile string | ||
} | ||
|
||
// GetDatafile returns a JSON string representation of the environment's datafile |
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.
same comment.
@@ -17,13 +17,21 @@ | |||
// Package config // | |||
package config | |||
|
|||
import "github.com/optimizely/go-sdk/pkg/entities" | |||
import ( |
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.
please update header.
@@ -62,17 +62,22 @@ func (s *OptimizelyConfigTestSuite) TestOptlyConfig() { | |||
s.Equal(s.expectedOptimizelyConfig.FeaturesMap, optimizelyConfig.FeaturesMap) | |||
s.Equal(s.expectedOptimizelyConfig.ExperimentsMap, optimizelyConfig.ExperimentsMap) | |||
s.Equal(s.expectedOptimizelyConfig.Revision, optimizelyConfig.Revision) | |||
|
|||
s.Equal(s.expectedOptimizelyConfig, *optimizelyConfig) |
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.
why removed?
pkg/config/static_manager_test.go
Outdated
@@ -48,7 +48,9 @@ func TestNewStaticProjectConfigManagerFromPayload(t *testing.T) { | |||
|
|||
func TestStaticGetOptimizelyConfig(t *testing.T) { | |||
|
|||
mockDatafile := []byte(`{"accountId":"42","projectId":"123","version":"4"}`) | |||
// since marshalling changes the order of keys in jsonString everytime, using json with a single key |
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.
why changed the payload, we are not supposed to change it.
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.
reverted this change.
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.
lgtm
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.
lgtm
Summary
Testing