-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[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
Conversation
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()) { |
There was a problem hiding this comment.
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?
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. |
@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. |
Alright, I'll split it tomorrow. |
@wouterj being different than |
…ly clear their own tokens
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. |
well, let's do it this way in this PR. I'll fix it in another PR sometime... |
Replaced by PR #2835 |