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

[Console] Command Lifecycle explications #5319

Closed
wants to merge 4 commits into from
Closed

[Console] Command Lifecycle explications #5319

wants to merge 4 commits into from

Conversation

94noni
Copy link
Contributor

@94noni 94noni commented May 26, 2015

Q A
Doc fix? yes
New docs? no
Applies to 2.3+
Fixed tickets #4996, #4366

Hi

I took the doc form https://github.com/symfony/symfony-demo/blob/master/src/AppBundle/Command/AddUserCommand.php by @javiereguiluz as a minimal explanations for this, which IMO could be beneficial for devs

@xabbuh
Copy link
Member

xabbuh commented May 31, 2015

Thank you for starting this @94noni. However, I think we should use a definition list for this:

Commands have three lifecycle methods:

:method:`Symfony\\Component\\Console\\Command\\Command::initialize`

    This method is executed before the ``interact()`` and the ``execute()``
    methods. It's main purpose is to initialize the variables used in the
    rest of the command methods.

:method:`Symfony\\Component\\Console\\Command\\Command::interact`

    This method is executed after ``initialize()`` and before ``execute()``.
    Its purpose is to check if some of the options/arguments are missing
    and interactively ask the user for those values.

:method:`Symfony\\Component\\Console\\Command\\Command::execute`

    This method is executed after ``interact()`` and ``initialize()``. It
    usually contains the logic to execute to complete this command task.

    Note that ``execute()`` is the only required method of the three. The
    ``initialize()`` and ``interact()`` methods are completely optional.

What do you think?

@94noni
Copy link
Contributor Author

94noni commented May 31, 2015

As you wish for formating
I can replace my pr by your code if its ok
Or close this and you open a new one

@wouterj
Copy link
Member

wouterj commented May 31, 2015

@94noni you can edit your branch in your fork to automatically update this PR. As you were the one starting this - and we are just maintainers - we would very much like it to have your name attached to these changes, so if you can update, please do so :)

@94noni
Copy link
Contributor Author

94noni commented Jun 1, 2015

Changes are done, ping @wouterj :)

:method:`Symfony\\Component\\Console\\Command\\Command::execute`

This method is executed after ``interact()`` and ``initialize()``. It
usually contains the logic to execute to complete this command task.
Copy link
Member

Choose a reason for hiding this comment

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

We need to remove "to complete" to make this readable I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok of course,
but It usually contains the logic to execute this command task. seems not good
What about It contains the logic you want the command executes ?

This method is executed after ``initialize()`` and before ``execute()``.
Its purpose is to check if some of the options/arguments are missing
and interactively ask the user for those values. This is the last place
where you can ask for missing options/arguments otherwise the command
Copy link
Member

Choose a reason for hiding this comment

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

I think we need a separation here:

[...] options/arguments otherwise [...]

[...] options/arguments. Otherwise [...]

@javiereguiluz
Copy link
Member

@94noni 👍 thanks for this addition! I've left some minor comments. They can probably be addressed by the documentation merger, so there is no need for you to update this PR. Thanks!

@OskarStark
Copy link
Contributor

@javiereguiluz so we can add the Finished-Label here?

@javiereguiluz
Copy link
Member

@OskarStark I don't know. Labeling issues is done only by our doc managers (@wouterj and @xabbuh).

wouterj added a commit that referenced this pull request Aug 14, 2015
This PR was squashed before being merged into the 2.3 branch (closes #5319).

Discussion
----------

[Console] Command Lifecycle explications

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.3+
| Fixed tickets | #4996, #4366

Hi

I took the doc form https://github.com/symfony/symfony-demo/blob/master/src/AppBundle/Command/AddUserCommand.php by @javiereguiluz  as a minimal explanations for this, which IMO could be beneficial for devs

Commits
-------

f45c392 [Console] Command Lifecycle explications
wouterj added a commit that referenced this pull request Aug 14, 2015
@wouterj
Copy link
Member

wouterj commented Aug 14, 2015

Thank you Antoine for adding this nice bit of missing docs! I've merged it into the docs and applied the comments by Javier and did some other minor tweaks in 01965cc

Also thanks to @OskarStark for the ping :)

@wouterj wouterj closed this Aug 14, 2015
@noniagriconomie
Copy link
Contributor

Fine ty :)
edit : Oups other account lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants