Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Updated the Quick Tour to the latest changes introduced by Symfony#5631

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

javiereguiluz
Copy link
Member

QA
Doc fix?yes
New docs?no
Applies toall
Fixed tickets-

This PR complements the pending PR#5497.

@@ -155,11 +155,12 @@ because that will be explained in the next section)::

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

You are right. I've added it to make it identical to the controller they will see. We decided to add theRequest import to make it easier for newcomers:https://github.com/symfony/symfony-standard/blob/2.8/src/AppBundle/Controller/DefaultController.php#L7

However, the content of theindexAction() is not the same as in the symfony-standard. I did this because in this case it's more confusing than useful.

In any case, let me know what you think about this and I'll make the changes you say. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if it makes things easier to understand for newcomers when they see ause statement for a class that is not used anywhere. Itcould give them the feeling that simply adding theuse statement triggers some magic. However, let's see what Ryan and Wouter think about it.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with@xabbuh here. I can see why it's usefull at the code side, I think it's confusion when doing it in the docs.

defined in the ``src/AppBundle/Controller/DefaultController.php`` file and rendered
the ``app/Resources/views/default/index.html.twig`` template. In the following
sections you'll learn in detail the inner workings of Symfony controllers, routes
and templates.
Copy link
Member

Choose a reason for hiding this comment

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

The new word wrapping in this paragraph is incorrect (the previous word wrapping was done by docbot)

@javiereguiluz
Copy link
MemberAuthor

I've changed everything you asked me to. This is ready for the final review. Thanks!

@xabbuh
Copy link
Member

👍

1 similar comment
@wouterj
Copy link
Member

👍

wouterj added a commit that referenced this pull requestSep 5, 2015
… by Symfony (javiereguiluz)This PR was squashed before being merged into the 2.3 branch (closes#5631).Discussion----------Updated the Quick Tour to the latest changes introduced by Symfony| Q             | A| ------------- | ---| Doc fix?      | yes| New docs?     | no| Applies to    | all| Fixed tickets | -This PR complements the pending PR#5497.Commits-------45b3781 Updated the Quick Tour to the latest changes introduced by Symfony
@wouterjwouterj closed thisSep 5, 2015
@DQNEO
Copy link
Contributor

👍

@javiereguiluzjaviereguiluz deleted the update_quick_tour branchMay 24, 2018 16:04
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@javiereguiluz@xabbuh@wouterj@DQNEO

[8]ページ先頭

©2009-2025 Movatter.jp