Skip to content

[WCM] Fix example code to reflect changes for #8226 #2741

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

Closed
wants to merge 1 commit into from

Conversation

alcaeus
Copy link
Contributor

@alcaeus alcaeus commented Jun 19, 2013

Q A
Doc fix? yes
New docs? PR symfony/symfony#8283
Applies to 2.1, 2.2, 2.3
Fixed tickets symfony/symfony#8226

// $this->securityContext->setToken(null);
// Make sure to only clear your token, not those of other authentication listeners.
// $token = $this->securityContext->getToken();
// if ($token !== null && $token instanceof WsseUserToken && $this->providerKey == $token->getProviderKey()) {
Copy link
Member

Choose a reason for hiding this comment

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

could you please do some line wrapping to avoid horizontal scrollbars?

@alcaeus
Copy link
Contributor Author

alcaeus commented Jun 19, 2013

In that case we should probably refactor the entire source code and trim all lines to ~80 characters. That's how many I can see when looking at the Symfony docs.
Also, this horizontal scrolling applies to pretty much every doc page.

@wouterj
Copy link
Member

wouterj commented Jun 19, 2013

@alcaeus well, it's one of our standards in the docs to try to avoid crossing the 80th character in code blocks. But I never did a PR to fix all examples.

However, are you sure the null check wasn't worth it? We can also split the condition on multiple lines.

@alcaeus
Copy link
Contributor Author

alcaeus commented Jun 19, 2013

Alright, I'll split it tomorrow.
The removed null check is per a suggestion in the Symfony PR, I merely updated the docs to reflect that change.

@stof
Copy link
Member

stof commented Jun 19, 2013

@wouterj being different than null is already catched by the instanceof check

@alcaeus
Copy link
Contributor Author

alcaeus commented Jun 22, 2013

WouterJ: I've checked - 80 characters really doesn't make any sense here, since even with splitting there are quite a few lines that break an 80 character gutter. Let me know if I should still go ahead and split as much as possible or if we'll just leave it like that for now.
I've also squashed my two commits in a single one for better readibility.

@wouterj
Copy link
Member

wouterj commented Jun 22, 2013

well, let's do it this way in this PR. I'll fix it in another PR sometime...

@alcaeus
Copy link
Contributor Author

alcaeus commented Jul 19, 2013

Replaced by PR #2835

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.

3 participants