Decouple trial from Split::Helper #286
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This refactoring further decouples the view helper from the inner workings and detials of starting an experiment. This completes the work needed to allow for an experiment to be started by an arbitrary class.
The goals that are being completed are not actually properties of the Trial object and should be used as context for completing a Trial from the helper.
The complexity and confusion of having multiple methods involved in the alternative choosing process replaced by one method that is guaranteed to only create side effects once.
Summary: Allow RedisAdapter to be used outside of a session context Remove unnecessary duplicate methods Add debugger gem Refactor finding experiments from Helper to ExperimentCatalog Refactor experiment starting process from Helper to Trial This refactoring further decouples the view helper from the inner workings and detials of starting an experiment. This completes the work needed to allow for an experiment to be started by an arbitrary class. This code is pretty rough and there is a lot of refactoring that this gem needs to make me happy. But this gets that process started and gets us the funtionality we need. All but one spec are currently passing. It's a spec we don't really care about for our own purposes, but I'll have it fixed before doing a PR to the main split fork. Reviewers: tyler Reviewed By: tyler Differential Revision: http://code.204.io/D122
1 similar comment
1 similar comment
|
An illustrated example of what this PR allows me to do. We have an experiment where we are split testing users to test how receiving a notification effects their long-term behavior. This runs in a background job and sends notifications to users who are not actually logged in (it's the current user's friends). ab_user = Split::Persistence.adapter.new(nil, user.tracking_id)
experiment = Split::ExperimentCatalog.find(:friend_post_notification)
trial = Split::Trial.new(user: ab_user, experiment: experiment)
send_notification(user) if trial.choose! |
|
This looks great @joshdover, thanks very much. I'll look at getting a new release out for this in a few days. 🎉 |
andrew
added a commit
that referenced
this pull request
Jan 3, 2015
Decouple trial from Split::Helper
|
@joshdover I've released this as part of v1.1.0 🎉 https://github.com/andrew/split/releases/tag/v1.1.0 |
This was referenced Jan 10, 2015
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This closes #285
A decent amount of refactoring done here, but it should clean things up quite a bit. No changes were made to the helper API and all of the specs are passing.