Skip to content

Conversation

aliabbasrizvi
Copy link
Contributor

No description provided.

@aliabbasrizvi
Copy link
Contributor Author

build-e2e

@coveralls
Copy link

coveralls commented Aug 21, 2018

Coverage Status

Coverage increased (+0.0003%) to 99.654% when pulling 088d9d4 on ali/fix_conversion_python_sdk into 210e9c6 on master.

@aliabbasrizvi
Copy link
Contributor Author

This includes commits from #135

event_dict[self.EventParams.TAGS] = event_tags

return snapshot
snapshot.setdefault(self.EventParams.EVENTS, []).append(event_dict)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to setdefault as opposed to just initializing an empty array with that event_dict?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, let me update that.

}

self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict))
self.config_dict_with_multiple_experiments = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be passed in for that one unit test that uses it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is ok

Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

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

I left some comments, mostly nits

@aliabbasrizvi
Copy link
Contributor Author

build-e2e

@mikeproeng37
Copy link
Contributor

build-e2e

@aliabbasrizvi aliabbasrizvi merged commit a85e7ba into master Aug 21, 2018
@aliabbasrizvi aliabbasrizvi deleted the ali/fix_conversion_python_sdk branch August 21, 2018 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants