8000 [Console] Command as service by gnugat · Pull Request #3621 · symfony/symfony-docs · GitHub
[go: up one dir, main page]

Skip to content

[Console] Command as service #3621

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

Merged
merged 9 commits into from
Mar 24, 2014
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Took @wouterj's advices into account
  • Loading branch information
Loïc Chardonnet committed Mar 1, 2014
commit a055140916455563182a2df5fe63acb2c6ad5a96
11 changes: 6 additions & 5 deletions cookbook/console/console_command.rst
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ By default, Symfony will take a look in the ``Command`` directory of you
bundles and automatically register your commands. For the ones implementing
the ``ContainerAwareCommand`` interface, Symfony will even inject the container.

If you wan to, you can instead register them as services in the container using
If you want to, you can instead register them as services in the container using
the ``console.command`` tag:

.. configuration-block::
Expand Down Expand Up @@ -115,11 +115,12 @@ the ``console.command`` tag:
.. tip::

Command as a service can be usefull in few situations:
Copy link
Contributor

Choose a reason for hiding this comment

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

usefull -> useful

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe even use the plural, commands as services, maybe even mention that is a best practice

Copy link
Member

Choose a reason for hiding this comment

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

commands as services isn't a best practise imo. The currently Console implementation lacks of a good support for it, it should only be used in specific cases. That's why this addition is so nice, it tells you a specific case to use it

Copy link
Author

Choose a reason for hiding this comment

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

@wouterj Precisely.

@cordoval You might be right about the plural, if we take a look at controllers, we can find the cookbook How to define Controllers as Services

* if you need your commands to be defined somewhere else than ``Command``
* if you need to access services or configuration parameters in the
``configure`` method

For example, Imagine you want to provide a default value for the ``name``
* If you need your commands to be defined somewhere else than ``Command``;
Copy link
Contributor

Choose a reason for hiding this comment

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

than in the? ... folder

Copy link
Contributor

Choose a reason for hiding this comment

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

lower the case at the start to be consistent with the second sentence below

* if you need to access services or configuration parameters in the
``configure`` method.

For example, imagine you want to provide a default value for the ``name``
option. You could hard code a string and pass it as the 4th argument of
``addArgument``, or you could allow the user to set the default value in the
Copy link
Contributor

Choose a reason for hiding this comment

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

in their

Copy link
Member

Choose a reason for hiding this comment

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

-1 "the configuration"

Copy link
Author

Choose a reason for hiding this comment

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

Should I change user to users?
The phrase: you could allow the user to set the default value in the configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wauter was saying that he is -1 about "the configuration" check my comment and also if you use user then use her configuration, although some genre sensitive people would ask you to either pluralize users and use their. However I still use the regular user-her dueto on my documentations 👶

Copy link
Member

Choose a reason for hiding this comment

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

@cordoval I was saying that I'm -1 for your change. "The configuration" is correct. (or at least, I tried to say that...)

However I still use the regular user-her dueto on my documentations

Sadly, this isn't your documentation and we changed our standards to be gender agnostic.

Copy link
Member

Choose a reason for hiding this comment

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

@gnugat to summarize: It's all perfect :)

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@gnugat i know that, that is why i said the above 👶

configuration.
Expand Down
0