Skip to content
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

Refactoring and passing ProjectConfig from Optimizely itself. #176

Merged
merged 1 commit into from
May 15, 2019

Conversation

aliabbasrizvi
Copy link
Contributor

@aliabbasrizvi aliabbasrizvi commented May 15, 2019

Summary

  • Refactoring to pass around ProjectConfig vs every class having its own copy of ProjectConfig. This way Bucketer, DecisionService and EventBuilder are static and makes way for us to introduce datafile manager.

Test plan

Unit tests and compatibility suite tests continue to pass.

@aliabbasrizvi aliabbasrizvi requested review from mikeproeng37, mjc1283 and a team May 15, 2019 01:22
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 99.781% when pulling 0257725 on ali/move_config into 5d3cee2 on master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 99.781% when pulling 0257725 on ali/move_config into 5d3cee2 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 99.781% when pulling 0257725 on ali/move_config into 5d3cee2 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 99.781% when pulling 0257725 on ali/move_config into 5d3cee2 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 99.781% when pulling 0257725 on ali/move_config into 5d3cee2 on master.

@@ -1960,23 +1963,23 @@ def test_get_enabled_features__broadcasts_decision_for_each_feature(self):
mock_variation_2 = opt_obj.config.get_variation_from_id('test_experiment', '111128')

def side_effect(*args, **kwargs):
feature = args[0]
feature = args[1]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This had to change as project_config is first param (index 0) now.

@aliabbasrizvi aliabbasrizvi merged commit 51fef17 into master May 15, 2019
@aliabbasrizvi aliabbasrizvi deleted the ali/move_config branch May 15, 2019 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants