-
Notifications
You must be signed in to change notification settings - Fork 29
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
[FSSDK-9781] warn on duplicate experiment key #343
[FSSDK-9781] warn on duplicate experiment key #343
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.
Looks good. A clarification.
@@ -91,6 +92,7 @@ def audiences_map | |||
|
|||
def experiments_map | |||
experiments_id_map.values.reduce({}) do |experiments_key_map, experiment| | |||
@logger.log(Logger::WARN, "Duplicate experiment keys found in datafile: #{experiment['key']}") if experiments_key_map.key? experiment['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.
Looks good. A question is the order in datafile is managed in experiment_id_map. Just for consistency, we're supposed to return the last experiment in the 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.
Yea Ruby iterates over hash values in insertion order, so the last instance of an experiment key will take precedence.
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
Test plan
Ticket
FSSDK-9781