Plan. Model, Events, Listeners. In Manager retrievePlans. Console command #14
Conversation
… command stripe:plan:update
|
Hi, Travis CI show error in php:hhvm, I found the follow issue, can you review it? Thanks you. |
|
Hi @webcome , I'm happy to see you again :) So, about the travi-ci error, I'll simply remove the hhvm test from the matrix, since it seems there is a problem on their own. So, if someone is using HVHVM, it should be aware of the problems and fix them on his own. Another last thing: can you add just a bit of documentation here? Just some lines to explain the basic of the use of Subscriptions and Plans and use of the |
|
@webcome , I've just fixed all unit tests. Please, pull to keep in synch with the |
|
Hi @Aerendir, I added a bit of documentation and synch with master and change requirement of phpunit to 5.* for passing the tests. I can see you change hhvm to nightly but the tests are not passing there. Can you review it? |
Command/StripeUpdatePlanCommand.php
Outdated
| $this | ||
| ->setName('stripe:update:plan') | ||
| ->setDescription('Update plans to your database.') | ||
| ->addOption('em', null, InputOption::VALUE_REQUIRED, 'The entity manager to use for this command.'); |
There was a problem hiding this comment.
Why do you let the user decide which EM to pass? Isn't it better to directly use the one created by the bundle?
Command/StripeUpdatePlanCommand.php
Outdated
| $aPlan = $plan->__toArray(); | ||
|
|
||
| $stripeLocalPlan = $em | ||
| ->getRepository("SerendipityHQ\\Bundle\\StripeBundle\\Model\\StripeLocalPlan") |
There was a problem hiding this comment.
Can you change this to StripeLocalPlan::class?
Syncer/PlanSyncer.php
Outdated
| * @param StripeLocalResourceInterface $localResource | ||
| * @param ApiResource $stripeResource | ||
| */ | ||
| public function syncLocalSources(StripeLocalResourceInterface $localResource, ApiResource $stripeResource) |
Command/StripeUpdatePlanCommand.php
Outdated
| @@ -0,0 +1,72 @@ | |||
| <?php | |||
There was a problem hiding this comment.
Why did you create a command to sync? Did you see I've improved the support of webhooks?
|
@webcome , yes, I dropped the tests for HHVM as is is currently incompatible with PHP 7 (from a link you posted above). So, in the meantime, we drop the support waiting for a support on their part. The tests currently fails on PHP 7.2 (the So, anyway I'll go to merge your PR as you did a very great job! Many many thanks! :) |
|
Just one last thing: if you see the checks of SensioInsights, you'll see a lot of issue with the update plans command. Maybe you can fix them in another PR... |
Hi,
I've added Plan. Model, Events, Listeners. In Manager retrievePlans. Console command stripe:plan:update for update plans.
If you can see well you will merge.
Thanks.