Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get beta version of this client library to stable. #4

Closed
9 tasks done
vtsao opened this issue Dec 20, 2013 · 46 comments
Closed
9 tasks done

Get beta version of this client library to stable. #4

vtsao opened this issue Dec 20, 2013 · 46 comments
Assignees
Milestone

Comments

@vtsao
Copy link
Contributor

vtsao commented Dec 20, 2013

Re-purposing this as a tracking bug for getting the beta version of this client library to stable.

Blocked by:

This was referenced Dec 20, 2013
@thierrymarianne
Copy link

Hey @vtsao, we have started to revise classes in this regard : https://github.com/theodo/adwords

@jxn
Copy link

jxn commented Jan 28, 2014

I agree on PSR-0. Note also that PSR-4 has now been approved as well: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-4-autoloader.md

@onema
Copy link

onema commented Apr 1, 2014

👍 Some PSR-0/PSR-4 love please!

@raul782
Copy link

raul782 commented Apr 1, 2014

That would be awesome, I'm deploying apps with php 5.5, and going back to use code w/o namespaces or compatible with 5.2 is a no no, Even php officially dx http://php.net/eol.php, 5.2 three years ago.
We should push towards PSR-0/1/2 at least and re-organize classes with their proper namespaces.
Maybe start a new repo and leave this as legacy code.

@saturnism
Copy link
Contributor

hey guys, have you all checked out the experimental branch w/ full namespace support?

@thierrymarianne
Copy link

Hey @saturnism, I did and I was pretty impressed by the good job. However, I had the feeling it was not totally compliant with latest autoloading standards (but I may be wrong about the latter):

see https://github.com/googleads/googleads-php-lib/blob/experimental/src/Google/Api/Ads/AdWords/v201402/AdGroupAdService.php#L31

ping @davinov

@vtsao
Copy link
Contributor Author

vtsao commented Oct 6, 2014

Thanks, we are still working on the experimental branch and deciding the path we want to take moving forward with it.

@vtsao vtsao assigned vtsao and unassigned saturnism Oct 6, 2014
@thierrymarianne
Copy link

@vtsao Thank you for the update, are these decision opened to discussion?

@henrikhj
Copy link

henrikhj commented Oct 9, 2014

Plus one on the namespaces. Without them, it's just a mess.
With PHP 5.2 being deprecated, there's no reason not to implement namespaces, to play nice with PSR-4.

@iknox
Copy link

iknox commented Jan 21, 2015

Any word on this (at the least an update of the experimental branch to work with the current API)? I believe Jan 1 was the official we-don't-support PHP 5.2 date was it not?

@vtsao
Copy link
Contributor Author

vtsao commented Jan 23, 2015

We are still planning to work on it, but no updates as of this moment.

@hakimio
Copy link

hakimio commented Mar 17, 2015

Why haven't there been any commits to experimental branch since august 2014? PSR-0 compliance these days is a must not something you can just ignore.
Just drop PHP 5.2 support already. It's been more than 4 years since 5.2.17 was released.

@joshhornby
Copy link

PSR-0 is deprecated is it not?

EDIT: http://www.php-fig.org/psr/psr-0/

@hakimio
Copy link

hakimio commented Mar 17, 2015

Yeah, I can see now that it is. PSR-4 now replaces it.

@vtsao
Copy link
Contributor Author

vtsao commented Mar 24, 2015

We've only recently announced that we're dropping PHP 5.2 support this Jan (many developers are still on 5.2 so it's hard to just stop supporting 5.2).

So, we are looking forward to revamping this client library this year. Stay tuned!

@onema
Copy link

onema commented Mar 25, 2015

👍

@vtsao
Copy link
Contributor Author

vtsao commented May 6, 2015

Yes, it's still on our radar.

@vtsao
Copy link
Contributor Author

vtsao commented May 6, 2015

I would recommend just using the non-experimental version for now. I hope to get around to making some updates to this this quarter.

@onema
Copy link

onema commented May 9, 2015

@florisgoumans it is not true that this library is "un-usable" in modern day frameworks.

We use it quite extensively making thousands of api calls for hundreds of clients every day in a symfony 2 application. We install it via composer without any issues and have access to all classes using the global namespace.

It is true that being a none PSR compliant library causes several pains (chief among them is the order in which you declare your object), but you should be able to use it without the use of requires or includes.

If you wrap the client functionality in a class or set of classes, updating the code in the future (with a PSR version of the library) will be quite easy!

Good luck

@onema
Copy link

onema commented May 10, 2015

@vtsao FYI I have created a small command/library that can be used to break files containing multiple classes into "single file per class", it can also add missing namespaces. Check it out: ClassyFile.

Generating the new files is as simple as :

include 'vendor/autoload.php';
$classifile = new \Onema\ClassyFile\ClassyFile();

// a namespace will be generated from the file path: `Google\Api\Ads\AdWords` and saved to the cwd under /Google/Api/Ads/AdWords/Lib/
$filePath = 'googleads/googleads-php-lib/src/Google/Api/Ads/AdWords/v201502/';
$classifile->generateClassFiles($filePath, true, 3, 4);

// a namespace will be generated from the file path: `Google\Api\Ads\AdWords\Lib` and saved to the cwd under /Google/Api/Ads/AdWords/Lib/
$filePath = 'googleads/googleads-php-lib/src/Google/Api/Ads/AdWords/Lib/';
$classifile->generateClassFiles($filePath, true, 3, 5);
// ...

or from the CLI:

$ php classyfile convert googleads/googleads-php-lib/src/Google/Api/Ads/Common/Lib/ --create-namespace --offset=3 --length=5

It currently doesn't account for adding use statements if none have been specified in the original file, but I believe is a good step in the right direction.

I have added two events to allow the extension of the library. Adding missing use statements could be added in the form of an event listener or an event subscriber.

Let me know if this is helpful or if you have any questions.

@hakimio
Copy link

hakimio commented May 11, 2015

@onema
Why does your library require "bing-ads-sdk-php" ?

@vtsao
Copy link
Contributor Author

vtsao commented Sep 19, 2016

FYI we've made another release of the beta client library to the experimental branch (1.4.0-beta). Please see:
https://github.com/googleads/googleads-php-lib/blob/experimental/CHANGELOG.md#140-beta

for changes.

@mzstic
Copy link

mzstic commented Oct 11, 2016

Hi, are there any updates when experimental branch will stop being experimantal?
Thx, looks good so far 👍

@vtsao
Copy link
Contributor Author

vtsao commented Oct 11, 2016

Thanks @mzstic. No updates at the moment, but keep following this issue!

@jbboehr
Copy link

jbboehr commented Nov 4, 2016

Are you planning to rename the package? 1.4 is less than 14.0...

@vtsao
Copy link
Contributor Author

vtsao commented Nov 7, 2016

@jbboehr Thanks for bringing this up, we're currently still thinking about how to proceed with this, will definitely let the community know when we've decided.

@dan-rally
Copy link

It appears the experimental branch now only supports a deprecated version of API, any updates on this please? Thanks.

@vtsao
Copy link
Contributor Author

vtsao commented Dec 20, 2016

@dan-rally we'll be updating experimental soon.

@dan-rally
Copy link

@vtsao OK thanks for the update.

@vtsao
Copy link
Contributor Author

vtsao commented Dec 22, 2016

The experimental library has now been pushed to stable and the old library has been deprecated. Please see our blog post announcement for details:
https://googleadsdeveloper.blogspot.com/2016/12/announcing-new-ads-php-client-library.html

Thanks for everyone's contributions to both the old and new libraries!

@vtsao vtsao closed this as completed Dec 22, 2016
@vtsao vtsao removed the beta label Dec 22, 2016
@vovan47 vovan47 mentioned this issue May 25, 2017
@Telrik Telrik mentioned this issue Jan 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests