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.

Sign up for GitHub

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

Jump to bottom

Added resource debug command #5458

Merged
merged 1 commit into from
Jul 10, 2016
Merged

Added resource debug command #5458

merged 1 commit into from
Jul 10, 2016

Conversation

dantleech
Copy link
Contributor

@dantleech dantleech commented Jul 7, 2016 •
edited
Loading

Q A
Bug fix? no
New feature? yes
BC breaks? no
Related tickets fixes #5191
License MIT

This PR adds a command to debug resources:

~/w/s/sylius ❯❯❯ ./app/console sylius:debug:resource
Select a resource to debug: 
  [0 ] sylius.order
  [1 ] sylius.order_item
  [2 ] sylius.order_item_unit
  ...
  [84] sylius.api_refresh_token
  [85] sylius.api_auth_code
  [86] sylius.payment_security_token
  [87] sylius.gateway_config
 > 42
+--------------------+------------------------------------------------------------+
| name               | promotion                                                  |
| application        | sylius                                                     |
| driver             | doctrine/orm                                               |
| classes.model      | Sylius\Component\Core\Model\Promotion                      |
| classes.repository | Sylius\Bundle\CoreBundle\Doctrine\ORM\PromotionRepository  |
| form.default       | Sylius\Bundle\CoreBundle\Form\Type\PromotionType           |
| form.choice        | Sylius\Bundle\ResourceBundle\Form\Type\ResourceChoiceType  |
| classes.interface  | Sylius\Component\Promotion\Model\PromotionInterface        |
| classes.controller | Sylius\Bundle\ResourceBundle\Controller\ResourceController |
| classes.factory    | Sylius\Component\Resource\Factory\Factory                  |
| default.0          | sylius                                                     |
+--------------------+------------------------------------------------------------+

The above should dumps all of the information in the metadata class. We could go further and show the routes associated with the resource, WDYT?

@dantleech dantleech changed the title Added resource debug command [WIPAdded resource debug command Jul 7, 2016
@dantleech dantleech changed the title [WIPAdded resource debug command [WIP] Added resource debug command Jul 7, 2016
dantleech
dantleech reviewed Jul 7, 2016
View reviewed changes
src/Sylius/Bundle/ResourceBundle/Command/DebugResourceCommand.php
public function configure()
{
$this->setName('sylius:debug:resource');
$this->addArgument('resource', InputArgument::OPTIONAL, 'Resource to debug');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add help, description

@dantleech
Copy link
Contributor Author

Need to add tests.

@pamil
Copy link
Contributor

pamil commented Jul 7, 2016

Looks awesome! 👍 🎉

@lchrusciel
Copy link
Member

I would be really useful! Nice. 👍

@lchrusciel
Copy link
Member

I think it would be enough to just list all resources after sylius:debug:resource call. Then, just make a sylius:debug:resource sylius.order to provide details. Just a symfony way.

@Zales0123
Copy link
Member

Primo: Nice feature, happy to see such an idea ;)
Secundo: Totally agreed with @lchrusciel about symfony-style commands

Zales0123
Zales0123 reviewed Jul 7, 2016
View reviewed changes
src/Sylius/Bundle/ResourceBundle/Command/DebugResourceCommand.php
@@ -0,0 +1,91 @@
<?php

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing license block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@pjedrzejewski pjedrzejewski added DX Issues and PRs aimed at improving Developer eXperience. New Feature labels Jul 7, 2016
@dantleech
Copy link
Contributor Author

dantleech commented Jul 7, 2016 •
edited
Loading

I think it would be enough to just list all resources after sylius:debug:resource call. Then, just make a sylius:debug:resource sylius.order to provide details.

I'm open to that, however, what advatages does that offer over the current approach? With the choice you only have to type a number (max 3 chars) to view the metadata, otherwise you will either need to memorize or copy and paste the name of the service and re-run the command.

@lchrusciel
Copy link
Member

I just matter of the default symfony2 behaviour. If we would go this way, it easier to grep some data or send a proper command to somebody. Also, I think it can be annoying to look for a proper resource between hundreds of them.

@dantleech
Copy link
Contributor Author

dantleech commented Jul 7, 2016 •
edited
Loading

I just matter of the default symfony2 behaviour. If we would go this way, it easier to grep some data

yeah grepping is a good argument, ofc we could act on the default --no-interaction option and show a list - but anyway, lets copy Symfony.

@dantleech dantleech force-pushed the resource_debug_command branch 3 times, most recently from 00bc1d1 to 56384d5 Compare July 8, 2016 09:06
@dantleech
Copy link
Contributor Author

Switched to using a standard list (no choice) and added tests. This good for me.

@dantleech dantleech changed the title [WIP] Added resource debug command Added resource debug command Jul 8, 2016
Zales0123
Zales0123 reviewed Jul 8, 2016
View reviewed changes
src/Sylius/Bundle/ResourceBundle/Command/DebugResourceCommand.php
*/
class DebugResourceCommand extends Command
{
private $registry;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are still plenty of docblocks missing in this file...

@dantleech
Copy link
Contributor Author

Added docblocks ..

@pjedrzejewski
Copy link
Member

Looks almost perfect, thank you very much Dan! Could you just add getParameters() to the interface? 👍

@dantleech
Copy link
Contributor Author

Updated :)

On Sun, Jul 10, 2016 at 03:20:40AM -0700, Paweł Jędrzejewski wrote:

Looks almost perfect, thank you very much Dan! Could you just add
getParameters() to the interface? 👍

—
You are receiving this because you authored the thread.
Reply to this email directly, [1]view it on GitHub, or [2]mute the thread.

Reverse link: [3]unknown

References

Visible links

  1. Added resource debug command #5458 (comment)
  2. https://github.com/notifications/unsubscribe/AAgZcZTACXVc4a6VGGnn9SLzxn2thUyiks5qUMd4gaJpZM4JG6AA
  3. Added resource debug command #5458 (comment)

@pjedrzejewski pjedrzejewski merged commit 9a8a7df into Sylius:master Jul 10, 2016
@pjedrzejewski
Copy link
Member

Awesome, this will be very useful! Cheers! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
DX Issues and PRs aimed at improving Developer eXperience.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[resource] Metadata debug command.
5 participants
@dantleech @pamil @lchrusciel @Zales0123 @pjedrzejewski