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

Update page_creation.rst #5644

Closed
wants to merge 1 commit into from
Closed

Update page_creation.rst #5644

wants to merge 1 commit into from

Conversation

jnadaud
Copy link

@jnadaud jnadaud commented Aug 20, 2015

Q A
Doc fix? yes
New docs? no
Applies to 2.7
Fixed tickets

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.7
| Fixed tickets |
@wouterj
Copy link
Member

wouterj commented Aug 20, 2015

Thanks for fixing even more occurences of this, Jérôme! I've patched this into the oldest, still maintained, version (which is 2.3 at the moment).

@wouterj wouterj closed this Aug 20, 2015
wouterj added a commit that referenced this pull request Aug 20, 2015
This PR was submitted for the 2.7 branch but it was merged into the 2.3 branch instead (closes #5644).

Discussion
----------

Update page_creation.rst

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.7
| Fixed tickets |

Commits
-------

1c8cfbe Update page_creation.rst
@jnadaud
Copy link
Author

jnadaud commented Aug 20, 2015

ok, thank you. Next time I will do it on 2.3 branch!

Best regards.

@@ -137,7 +137,7 @@ You can even shorten this with the handy :class:`Symfony\\Component\\HttpFoundat
// --> don't forget this new use statement
use Symfony\Component\HttpFoundation\JsonResponse;

class LuckyController
class LuckyController extends Controller
Copy link
Member

Choose a reason for hiding this comment

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

well, this example does not depend on any method of the base class. So why extending it ?

Copy link
Member

Choose a reason for hiding this comment

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

As this article describes how to create one action. So each code example is based on the previous example. It would be very strange to have the extends part in all examples but one.

Also, as this article is focused on beginners, it's confusion when not extending Controller imo.

Copy link
Member

Choose a reason for hiding this comment

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

@wouterj I'd like to ask you something about this LuckyController. What do you think about renaming it to DefaultController? This code is shown very early in the book and it's linked from the new Symfony Welcome Page, so most of the readers will be newcomers.

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.

4 participants