prismicio-community / php-kit

Community maintained development kit for Prismic and the PHP language
https://prismic.io
Other
109 stars 83 forks source link

[wip] standardize code #2

Closed rande closed 11 years ago

rande commented 11 years ago

Contributor Works

Prismic Works

rudyrigot commented 11 years ago

Hey, it looks like you mean to help, and that's awesome! But I'm not sure you requested your prismic.io beta invite yet, which is a shame! Should I simply invite you on your GMail account?

Dinduks commented 11 years ago

@rudyrigot: @srenault took care of that.

Thanks for the help @rande.
By the way, Ruby kit tests are more complete, it'd be more efficient to use them instead of Python kit's in my opinion.

srenault commented 11 years ago

@rudyrigot I already invite @rande (See the issue #1). Thank for @rande for his pull request. We will take care of it.

rudyrigot commented 11 years ago

Ok, cool, thanks guys! :)

srenault commented 11 years ago

Hey @rande, I think we need your pull request to go forth with the PHP kit. That's why, I prefer to wait to merge others pull requests before merging yours. As soon as possible, I will check the work you already did and merge it. For the rest of the todo list, you can submit another pull requests.

rande commented 11 years ago

I have completed the first part of the migration. However this need to be checked against the demo to make sure there is no issue. The API has been change a bit to introduce a Response object. the API is mainly an entry to the prismic API.

Some code from the php-starter-kit should be move here (like the query code or the context)

srenault commented 11 years ago

Ok cool ! However, please don't move the code about query & context from php-starter-kit to php-kit. Let me know if I can help you in anything.

rande commented 11 years ago

What is the need of $maybeAccessToken in the Api::get ? as we can use the accessToken from the constructor

rande commented 11 years ago

There is an issue with the current code, the access_token is not passed to the SearchForm::submit method.

I manage to retrieve the content by hard coding some value and doing :

$home = $app['prismic']->get($app['prismic.api']); // Silex App

$results = $home->forms()->everything
     ->ref('Uk0npknM070GB-1L')
     ->query('[[:d = at(document.type, "event")]]')
     ->submit();

From what I understand from the code, you try to have a common api across language, ie: being able to do ctx.home.forms.everything ... is that correct ?

I haven't find out how to retrieve the ref code for the repository. I get it from the api browser.

srenault commented 11 years ago

"What is the need of $maybeAccessToken in the Api::get ? as we can use the accessToken from the constructor" => Because Api::get is a static method ?

"There is an issue with the current code, the access_token is not passed to the SearchForm::submit method." => In fact, when you call :

$ctx->api->forms()->everything->ref($ctx->ref)->submit();

The forms function add the token in the data.

"ctx.home.forms.everything ... is that correct ?" => Yes, it's correct.

"I haven't find out how to retrieve the ref code for the repository" => The endpoint /api give you the "master" ref and others releases ref (if you give a token).

To get the master ref, you can do the following:

Api::get("https://lesbonneschoses.prismic.io/api")->master()->ref;
rande commented 11 years ago

Ok so I break the api by adding a new Response object. The Api object was too coupled but I guess it something you want. Why do you need a static method get ? Le 4 oct. 2013 19:22, "Sébastien RENAULT" notifications@github.com a écrit :

"What is the need of $maybeAccessToken in the Api::get ? as we can use the accessToken from the constructor" => Because Api::get is a static method ?

"There is an issue with the current code, the access_token is not passed to the SearchForm::submit method." => In fact, when you call :

$ctx->api->forms()->everything->ref($ctx->ref)->submit();

The forms function add the token in the data.

"ctx.home.forms.everything ... is that correct ?" => Yes, it's correct.

"I haven't find out how to retrieve the ref code for the repository" => The endpoint /api give you the "master" ref and others releases ref (if you give a token).

To get the master ref, you can do the following:

Api::get("https://lesbonneschoses.prismic.io/api")->master()->ref;

— Reply to this email directly or view it on GitHubhttps://github.com/prismicio/php-kit/pull/2#issuecomment-25715594 .

srenault commented 11 years ago

I think it is a more comfortable api. You've right by saying we want all prismic.io kits to have the same api. Making this php library to be more standard is our goal here. That's why I will ask you to not change the API. Anyway, thank you very much for you effort !

damienalexandre commented 11 years ago

On the behalf of all the PHP dev's out-there, thanks Thomas \o/

rande commented 11 years ago

This seems to be ok on my side. Now it need to be tested with real life project to fix any regressions.

srenault commented 11 years ago

Hey, I test it on the php-starter-kit and it seems to be ok ! I just made some little fixes. I see you made a commit 14 minutes ago. Do you think you have others commits to do ?

rande commented 11 years ago

No, I will open others PR if required

srenault commented 11 years ago

Ok, again thank you very much for you help.

rande commented 11 years ago

@srenault thanks. Now you need to register the package to packagist.org and add a post commit hook from github to packagist. The same must be done for travis. So PR can be remotely tested before merging ...