-
Notifications
You must be signed in to change notification settings - Fork 32
Refactor experiment and events getters #179
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
Conversation
wangjoshuah
commented
Apr 12, 2018
- Remove throws declarations on methods since we never actually throw.
- Change docs to explain that we send it to the error handler instead
…Change docs to explain that we send it to the error handler instead
Pull Request Test Coverage Report for Build 504
💛 - Coveralls |
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, just a few small comments
getExperimentKeyMapping() | ||
.get(experimentKey); | ||
|
||
// if the given experiment key isn't present in the config, log and potentially throw an exception |
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.
Revise comment on throwing error
getEventNameMapping() | ||
.get(eventName); | ||
|
||
// if the given event name isn't present in the config, log and potentially throw an exception |
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.
No exceptions anymore!