jdolitsky / AppDotNetPHP

PHP library for the App.net Stream API
34 stars 19 forks source link

Extended Post options #14

Closed ghost closed 12 years ago

ghost commented 12 years ago

Any plans to update the lib to support passing the new @mentions flag and the other post options not implemented? I can do it but if someone else can that's good too. Thinking there will be a few API updates this week.

ghost commented 12 years ago

include_muted
include_deleted include_directed_posts include_user (Coming soon) include_annotations (Coming soon)

ravisorg commented 12 years ago

Ok, this deserves a conversation with @jdolitsky I think. It can probably be safely assumed that they're going to keep adding general parameters to the API. Continuing to add parameters to the functions isn't going to scale. We can easily fix this, but it won't be backward compatible. Instead of a bunch of parameters passed, pass a single associative array with key=>value pairs. This would look something like:

$app->getUserStream(array(
  'count'=>10,
  'before_id'=>15543,
  'include_muted'=>true,
  etc...
));

This would only be for optional parameters. For example, getPostReplies requires the post id, so it would look something like:

$app->getPostReplies(123,array(
  'count'=>10,
  'before_id'=>15543,
  'include_muted'=>true,
  etc...
));

In other words, required parameters always come first, and you can always leave off the general parameters if you don't need it:

$app->getPostReplies(123);

Thoughts?

ghost commented 12 years ago

looks great to me, i've used this method happily before. I don't mind the upgrade, better to get the niggly bits taken care of properly now.

Harold http://hxf148.com

On Wed, Aug 29, 2012 at 10:03 AM, ravisorg notifications@github.com wrote:

Ok, this deserves a conversation with @jdolitskyhttps://github.com/jdolitskyI think. It can probably be safely assumed that they're going to keep adding general parameters to the API. Continuing to add parameters to the functions isn't going to scale. We can easily fix this, but it won't be backward compatible. Instead of a bunch of parameters passed, pass a single associative array with key=>value pairs. This would look something like:

$app->getUserStream(array( 'count'=>10, 'before_id'=>15543, 'include_muted'=>true, etc...));

This would only be for optional parameters. For example, getPostReplies requires the post id, so it would look something like:

$app->getPostReplies(123,array( 'count'=>10, 'before_id'=>15543, 'include_muted'=>true, etc...));

In other words, required parameters always come first, and you can always leave off the general parameters if you don't need it:

$app->getPostReplies(123);

Thoughts?

— Reply to this email directly or view it on GitHubhttps://github.com/jdolitsky/AppDotNetPHP/issues/14#issuecomment-8126181.

ravisorg commented 12 years ago

@jdolitsky it's your library, thoughts on maintaining backward compatibility at this stage?

jdolitsky commented 12 years ago

hey, i think that this is definitely the way to go considering they might be making tons of changes to the API. I had thought about doing this from the start. perhaps make another branch "old code" then push these changes? @ravisorg thanks for the commit today and handling issues etc. this morning. ive been in class and keep getting these github emails to my phone and cant do anything about it lol

also we should update readme to reflect these changes as well as changes to getAuthUrl On Aug 29, 2012 10:08 AM, "ravisorg" notifications@github.com wrote:

@jdolitsky https://github.com/jdolitsky it's your library, thoughts on maintaining backward compatibility at this stage?

— Reply to this email directly or view it on GitHubhttps://github.com/jdolitsky/AppDotNetPHP/issues/14#issuecomment-8126356.

ravisorg commented 12 years ago

Created a branch for the old code if people want it (better to tag, but I'm not a git expert and couldn't get the bloody thing working).

Submitted untested code to a new branch: https://github.com/jdolitsky/AppDotNetPHP/tree/new_parameter_style

If someone wants to try it out to make sure it works, I'd appreciate it, otherwise I'll get to it as soon as I can.

ravisorg commented 12 years ago

...also updated the readme for the new getAuthUrl paramters

ghost commented 12 years ago

Planning to test/update in 12-24 hours, will report. Thanks for updating docs.

cheers,

Harold http://hxf148.com

On Wed, Aug 29, 2012 at 3:25 PM, ravisorg notifications@github.com wrote:

...also updated the readme for the new getAuthUrl paramters

— Reply to this email directly or view it on GitHubhttps://github.com/jdolitsky/AppDotNetPHP/issues/14#issuecomment-8137790.

ravisorg commented 12 years ago

Excellent, thank you, much appreciated :)

jdolitsky commented 12 years ago

looks good to me, fixed some stuff w/ EZ and pushed changes

ghost commented 12 years ago

Appeio is using latest, looks good. Thanks!

Harold http://hxf148.com

On Sun, Sep 2, 2012 at 5:26 AM, Josh Dolitsky notifications@github.comwrote:

looks good to me, fixed some stuff w/ EZ and pushed changes

— Reply to this email directly or view it on GitHubhttps://github.com/jdolitsky/AppDotNetPHP/issues/14#issuecomment-8220296.