Skip to content

New Feature: Change the Default Command in the Console component#3426

Merged
weaverryan merged 9 commits intosymfony:masterfrom
dcsg:ticket_9776
Jan 9, 2014
Merged

New Feature: Change the Default Command in the Console component#3426
weaverryan merged 9 commits intosymfony:masterfrom
dcsg:ticket_9776

Conversation

@dcsg
Copy link
Copy Markdown
Member

@dcsg dcsg commented Jan 4, 2014

Q A
Doc fix? no
New docs? yes (symfony/symfony#9776)
Applies to 2.5+
Fixed tickets

I followed the suggestion from @wouterj in IRC on changing the single_command_tool and added the documentation for the new feature there.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you should remove this file comment, it's symfony specific and that's not something we do in the component docs.

@wouterj
Copy link
Copy Markdown
Member

wouterj commented Jan 5, 2014

You should also keep the previous file and say "This article is moved to ...", otherwise we get lots of broken links.

@dcsg
Copy link
Copy Markdown
Member Author

dcsg commented Jan 5, 2014

@wouterj thanks for your review!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This does aply to the section started on line 16, not to the hole article, does it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes true! will move it to the right section

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

missing comma after Fortunately I think

@dcsg
Copy link
Copy Markdown
Member Author

dcsg commented Jan 5, 2014

@xabbuh @wouterj applied your suggestions! Thanks for the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should stay in a separate page, as it is a totally separate feature than changing the default command

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, I think the best solution is to keep it in its original location. It may be referenced from the page about changing the default command, to wanT than changing the default is not the best way if you only want 1 command, but it should not be in the same page IMO. It makes it harder to find the feature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with @stof

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1, and it would also solve the redirect_map problem I think :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants