Add support for preview API.

[jmhobbs]

Alternative approach to solve #1

[incraigulous]

Thanks @jmhobbs! Sorry it's taken so long to respond! Your code looks good. Have you been using this in production? Any issues you're aware of?

[incraigulous]

Also, I see that this is an alternative to #13. Mind if I ask what your argument is on using this method instead? I agree the implementation is much simpler, but I like the idea the preview sdk being it's own object instead of piggybacking off of the content sdk.

I'm assuming you found it easier when using the sdk to simply pass in a boolean instead of having to construct a different object? I think that's a valid argument, but I'm worried out muddying up the interface by throwing another parameter into the constructor. I would love to hear your thoughts.

[jmhobbs]

To be honest, I wrote it without knowing #13 existed :)

I was just going for the bare minimum changes to avoid regressions. I'd be
happy to rework it as its own object if you like, it wouldn't take much
change.

Not using it in production yet, I just wanted to be able to see drafts in
the dev environment I'm working on.

On Wednesday, December 30, 2015, Craig Wann [email protected]
wrote:

Also, I see that this is an alternative to #13
#13. Mind if I ask
what your argument is on using this method instead? I agree the
implementation is much simpler, but I like the idea the preview sdk being
it's own object instead of piggybacking off of the content sdk.

I'm assuming you found it easier when using the sdk to simply pass in a
boolean instead of having to construct a different object? I think that's a
valid argument, but I'm worried out muddying up the interface by throwing
another parameter into the constructor. I would love to hear your thoughts.

—
Reply to this email directly or view it on GitHub
#13 (comment)
.

• John Hobbs

[email protected]

http://www.velvetcache.org/

[incraigulous]

Copy that! I guess it wouldn't make since to use a preview api in "production" huh? :-) I just mean are you using it on an active product. Let me chat with Marcgoosen and get his thoughts. I think he's been using his branch for a while. I think he actually dried up my code a bit, so I like his implementation. Mind taking a look at his branch out to see if there is anything he missed that you need?

[jmhobbs]

Yeah, it's in use right now, no issues so far :)

I see the benefit of just passing around a client class, makes sense. The #13 branch just seems kind of messy, it's got other changes mixed into it too.

Whichever you choose is fine by me, I just wanted to offer up what I had.

[incraigulous]

OK, let me take a look again.

[incraigulous]

Yeah, I don't mind him adding the locale limiter. The other off target change I see is adding the ability to pass an assoc value to the json request results, but it looks like he took that out. I haven't actually pulled down the branch yet, I'm just looking at the commit history, so I could be wrong. If he didn't, I'll take it out before it goes live. I'm not sure if that's how I would want to handle that. I think I would actually be more in favor of chaining an ->assoc() method if that's actually functionality people need. I think that would be best added in a different pull request though.

[incraigulous]

By the way, I really appreciate your pull request and thoughts! Keep em coming!