Skip to content
This repository has been archived by the owner on Sep 28, 2023. It is now read-only.

Headline testing via Experiment API #1

Merged
merged 19 commits into from
May 16, 2014
Merged

Conversation

thatsjonsense
Copy link
Contributor

It works, but the code's not very pretty.

@eric-optimizely @arthuracs @ramaranganath

@thatsjonsense
Copy link
Contributor Author

@eric-optimizely @arthuracs fyi, here's the latest wp plugin code. Once Neha and Dogan's fixes are in (https://github.com/optimizely/optimizely/pull/2322 and https://github.com/optimizely/optimizely/pull/2315) I'd like to release this, at least on Github and developers.optimizely.com.

Before updating our current WP plugin with this, we also need a token generation page.

@thatsjonsense
Copy link
Contributor Author

@dogaan @nehasingla @eric-optimizely @NikhilChelliah if anyone has time today, here's the code i'd like to put up for the WP plugin. Most of the magic happens in optimizely.js, config.js, and edit.js.

@eric-optimizely
Copy link

Adding @jordangarcia and @ramaranganath who I know have more recent PHP experience and could probably help to give a more solid review.

$project_code = stripcslashes($_POST['project_code']);
$variation_template = stripcslashes($_POST['variation_template']);

if ( empty($token) ) {

Choose a reason for hiding this comment

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

Just as a head's up, "empty" in PHP is rather inclusive. The following are considered empty:

"" (an empty string)
0 (0 as an integer)
0.0 (0 as a float)
"0" (0 as a string)
NULL
FALSE
array() (an empty array)
$var; (a variable declared, but without a value)

Just makes sure this jibes with what you want to do here.

@ramaranganath
Copy link

I looked through the PHP, didn't see anything horrendously broken. I'm sure it could be cleaned up some, but I'm guessing this is just to get things to work, not something to do a lot of maintenance on.

@thatsjonsense
Copy link
Contributor Author

@asaschachar

@thatsjonsense
Copy link
Contributor Author

Ok, I'm merging this in since people have been asking and I don't think the code needs to be perfect. I did take @eric-optimizely 's feedback of putting inline comments in wherever the API's used, and @ramaranganath 's feedback of keeping num_variations in a constant.

If anyone would like to take another pass on this in the future that would still be great!

thatsjonsense pushed a commit that referenced this pull request May 16, 2014
Headline testing via Experiment API
@thatsjonsense thatsjonsense merged commit 8ccc90e into master May 16, 2014
@bhamodi bhamodi deleted the headline-testing branch May 15, 2015 22:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants