Using composer and PSR-4 namespaces#12
Conversation
This is needed to provide this as a library on packagist.
| $response = $this->_get(Killbill_Client::PATH_CATALOG, $headers); | ||
| $this->fullCatalog = json_decode($response->body); | ||
| $response = $this->_get(Client::PATH_CATALOG, $headers); | ||
| $this->fullCatalog = json_decode($response->body)[0]; |
There was a problem hiding this comment.
Not sure about this one, tbh. The API returns an array here.
There was a problem hiding this comment.
In case of multi-versioned catalogs, multiple versions will be returned (so we need to keep the array).
There was a problem hiding this comment.
Alright, so it is possible to get multiple catalogs here? This should probably be reflected in the PHP client in some way.
There was a problem hiding this comment.
Alright, so it is possible to get multiple catalogs here?
Yes, in most cases actually you would have a list of historical catalogs.
This should probably be reflected in the PHP client in some way.
👍
phpunit 5 doesn't support PHP 5.4
pierre
left a comment
pierre
left a comment
There was a problem hiding this comment.
This pull request strives to modernize the whole Killbill PHP client. Composer is a dependency manager for PHP and widely-used. To be able to require this client via composer, it is needed to submit the package to Packagist - which then just points to the Github Repo.
Ack, we'll create an "official" Kill Bill account and set it up. Any best practice recommended for releases and versioning? For example, we leverage SemVer in other clients to have different release trains for different Kill Bill versions (these changes will require Kill Bill 0.18.x).
There's a lot that has changed with the library, but the changes are mostly due to the usage of namespaces. From the logic itself, basically nothing changes.
Looks good AFAICT, I'll let @sbrossie take a look as well.
Pinging @timrayers in case he'd like to review too, since he submitted the changes for 0.16.x.
Also, not all of the unit tests are working. However, those didn't work before either.
Feel free to reach out if we can help (I believe Kill Bill needs the SpyCarAdvanced catalog for these tests).
If this pull request gets accepted, we'd love to do some more changes to this library to "modernize" it.
Thanks! Very much appreciated! 😍
| 1. Download the library and unzip it in your application | ||
| 2. Require the `lib/killbill.php` script | ||
| 3. Point the library to your Killbill instance via the `Killbill_Client::$serverUrl` variable (http://127.0.0.1:8080 by default) | ||
| 1. Require the library via [composer](https://getcomposer.org): `composer require killbill\killbill-client` |
There was a problem hiding this comment.
Is there any new requirement regarding the minimal PHP version supported by the library? If so, we should probably add it in the README.
There was a problem hiding this comment.
No, this should run under PHP 5.3. However, i would probably drop the support for those older versions as they are no longer officially supported and don't get any updates.
|
|
||
| In order to user the library, you need to: | ||
|
|
||
| 1. Download the library and unzip it in your application |
There was a problem hiding this comment.
Could we keep some instructions for non-composer users? I can think of at least two deployments on legacy codebases that aren't using any package manager (code is checked-in and updated manually in the app tree).
There was a problem hiding this comment.
I'm not sure about this. What composer does is to create an autoload file, which registers an autload function. This could probably be done manually. However, this wouldn't make too much sense.
It is really easy to use composer in a non-composer project. Just use composer to download the dependency and use it's auto-generated autoload script. The complete vendor/ folder can even be checked into version control, so that no additional step is required while deploying.
What do you think @pierre?
There was a problem hiding this comment.
I think just having the steps and commands to use composer in a non-composer project would be fine.
| $response = $this->_get(Killbill_Client::PATH_CATALOG, $headers); | ||
| $this->fullCatalog = json_decode($response->body); | ||
| $response = $this->_get(Client::PATH_CATALOG, $headers); | ||
| $this->fullCatalog = json_decode($response->body)[0]; |
There was a problem hiding this comment.
In case of multi-versioned catalogs, multiple versions will be returned (so we need to keep the array).
| case CURLE_COULDNT_RESOLVE_HOST: | ||
| case CURLE_OPERATION_TIMEOUTED: | ||
| throw new Exception("Failed to connect to Killbill: " . $message); | ||
| throw new \Exception("Failed to connect to Killbill: " . $message); |
There was a problem hiding this comment.
Why the backslash (PHP noob here 😺)?
There was a problem hiding this comment.
This is because of the usage of namespaces. If we'd just write Exception (without the backslash) here, the autloader would search for Killbill\Client\Exception.
|
@MaxGfeller Looks good on my end. I believe there are a few things that @pierre mentioned and that should be addressed. We'll take a look at |
|
Packagist account created, but we'll need to merge the PR first before being able to submit the repo. |
|
Awesome, thank you for the feedback @pierre @sbrossie !
Composer itself works best with SemVer. Not sure what the best way is, but you could probably do it similar to the ruby or java clients.
Yes, i'm kinda confused by those :) Apparently the PHP client tests use the SpyCarBasic or SpyCarAdvanced catalogs, but i'm not really sure where they get set up. Maybe you could guide me in the right direction here. |
Easiest is to use the start-server script ( Alternatively, you could upload the catalog manually (through cURL or Kaui) |
|
Alright, i've enhanced the readme with a section about using the lib in non-composer environments and reverted the change with the catalogs. The tests were failing before so i'd fix that in another pull request, if that's ok with you. I'd also love to change the setup a bit so the tests don't require a live Killbill instance and could also be run on Travis. |
Thanks!
👍 FWIW, in other client libraries, we have 2 sets of tests: a unit suite and an integration suite (former run by Travis, latter requiring Kill Bill and run manually). |
|
Packagist has been setup. |
|
Awesome, thanks for merging! |
3 participants
This pull request strives to modernize the whole Killbill PHP client. Composer is a dependency manager for PHP and widely-used. To be able to require this client via composer, it is needed to submit the package to Packagist - which then just points to the Github Repo.
Composer also enables us to use proper namespaces, that are defined by a community standard: PSR-4.
There's a lot that has changed with the library, but the changes are mostly due to the usage of namespaces. From the logic itself, basically nothing changes.
Also, not all of the unit tests are working. However, those didn't work before either.
If this pull request gets accepted, we'd love to do some more changes to this library to "modernize" it.