Skip to content

Feature/event listener #88

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

Merged
merged 47 commits into from
Nov 21, 2017
Merged

Conversation

thomaszurkan-optimizely
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 95.644% when pulling 170de07 on feature/eventListener into 4cbf7c5 on master.

@coveralls
Copy link

coveralls commented Oct 19, 2017

Coverage Status

Coverage decreased (-0.3%) to 95.644% when pulling 61003b3 on feature/eventListener into 4cbf7c5 on master.

@coveralls
Copy link

coveralls commented Oct 19, 2017

Coverage Status

Coverage decreased (-0.2%) to 95.733% when pulling 13937fd on feature/eventListener into 4cbf7c5 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 96.577% when pulling 56e9275 on feature/eventListener into 4cbf7c5 on master.

""" NotificationTypes for the notification_center.NotificationCenter
format is NOTIFICATION TYPE: list of parameters to callback.
"""
ACTIVATE = "ACTIVATE:experiment, user_id,attributes, variation, event"
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the reason for changing this back to ACTIVATE from DECISION? ACTIVATE is more of an Optimizely term and might not make sense to developers of other anaytics products. Instead DECISION is a more industry-standard term.

Copy link
Contributor

Choose a reason for hiding this comment

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

@thomaszurkan-optimizely I left a comment here in case you miss it

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 96.419% when pulling caf64a7 on feature/eventListener into 4cbf7c5 on master.

@coveralls
Copy link

coveralls commented Nov 14, 2017

Coverage Status

Coverage increased (+0.5%) to 96.422% when pulling 650d04d on feature/eventListener into 4cbf7c5 on master.

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.

Ship it

@coveralls
Copy link

coveralls commented Nov 15, 2017

Coverage Status

Coverage decreased (-0.5%) to 96.422% when pulling 8d4c3b2 on feature/eventListener into ebb0ad7 on master.

if decision.experiment is not None and \
decision.experiment.audienceIds is not None and \
len(decision.experiment.audienceIds) > 0:
audience_id = decision.experiment.audienceIds[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

@thomaszurkan-optimizely Hi, I have a question here. In the case where the rollout rule/experiment has multiple audience IDs, and the user didn't qualify for the first audience, but for some other audience id in the list. Do we still send the audience at index 0 in send_notifications method?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 96.422% when pulling 3631a0a on feature/eventListener into ebb0ad7 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 96.422% when pulling c8f2f75 on feature/eventListener into ebb0ad7 on master.

@coveralls
Copy link

coveralls commented Nov 17, 2017

Coverage Status

Coverage decreased (-0.5%) to 96.422% when pulling a86bc82 on feature/eventListener into ebb0ad7 on master.

@coveralls
Copy link

coveralls commented Nov 18, 2017

Coverage Status

Coverage decreased (-0.03%) to 96.869% when pulling 65a180c on feature/eventListener into ebb0ad7 on master.

@coveralls
Copy link

coveralls commented Nov 20, 2017

Coverage Status

Coverage decreased (-0.03%) to 96.869% when pulling e10389f on feature/eventListener into ebb0ad7 on master.

@coveralls
Copy link

coveralls commented Nov 21, 2017

Coverage Status

Coverage increased (+0.1%) to 97.027% when pulling ea13f7b on feature/eventListener into ebb0ad7 on master.

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.

lgtm, please wait for e2e tests to pass

@thomaszurkan-optimizely thomaszurkan-optimizely merged commit c98a162 into master Nov 21, 2017
@aliabbasrizvi aliabbasrizvi deleted the feature/eventListener branch December 11, 2017 19:05
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.

5 participants