symfony/symfony-docsPublic

NotificationsYou must be signed in to change notification settings
Fork5.3k
Star2.3k

Update security.rst#6637

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

Jump to bottom

Closed

norbert-n wants to merge2 commits intosymfony:2.3fromnorbert-n:patch-1

Closed

Update security.rst#6637

norbert-n wants to merge2 commits intosymfony:2.3fromnorbert-n:patch-1

Conversation

Copy link

Contributor

norbert-n commentedJun 5, 2016

missing use statement

Update security.rst

0ba15ed

Copy link

Contributor

OskarStark commentedJun 7, 2016

LGTM 👍

xabbuh reviewed

Jun 8, 2016

View reviewed changes

book/security.rst


		use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
		use Symfony\Component\HttpFoundation\Response;
		use Symfony\Bundle\FrameworkBundle\Controller\Controller;

Copy link

Member

Choose a reason for hiding this comment

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

Can you please sort theuse statements alphabetically?

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

done

use statements alphabetically

e350dd5

Copy link

Member

javiereguiluz commentedJun 10, 2016

It looks good ... but I don't know if doc maintainers will agree with it. The// ... at the beginning of the code is used in the Symfony Docs to refer to"all the "use" that are needed but we don't show them here because they are very basic or they were showed in the previous examples".

Copy link

Member

wouterj commentedJun 10, 2016

Personally, I only apply// ... to the "shown before" standard, not the "very basic". For beginners, nothing is basic in Symfony. So a 👍 for me.

wouterj added the Status: Reviewed label

Jun 10, 2016

Copy link

Member

xabbuh commentedJun 11, 2016

I absolutely agree with@wouterj. This is the book which is probably one of the first parts newcomers are reading. And secondly this is the first controller example in this chapter. So 👍 from me too.

xabbuh added a commit that referenced this pull request

Jun 11, 2016

minor#6637Update security.rst (norbert-n)

a94d9f2

This PR was submitted for the 2.3 branch but it was merged into the 2.7 branch instead (closes#6637).Discussion----------Update security.rstmissing use statementCommits-------5ac5d60 Update security.rst

Copy link

Member

xabbuh commentedJun 11, 2016

Thank you@norbert-n. I have merged your fix into the2.7 branch (that's why it's shown as closed instead of merged) as the2.3 branch is no longer maintained.