-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
add missing crosslinks between authorization and voters #3120
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
update to latest
Thanks for starting this! Really appreciated. However, make sure you break the lines after the first word that crosses the 72th character and maybe change |
@wouterj alright, I changed them to seealso:: and I checked each seealso:: and each has a prepended blank line. |
|
||
.. seealso:: | ||
|
||
You can change the default strategy in the configuration: :ref:`book-security-voters-changing-the-access-decision-strategy` |
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.
I would use configuration
as a label for the link:
.. seealso::
You can change the default strategy in the :ref:`configuration <book-security-voters-changing-the-access-decision-strategy>`.
I updated the pull request according to @xabbuh suggestions. |
|
||
.. seealso:: | ||
|
||
You can change the default strategy in the :ref:`configuration <book-security-voters-changing-the-access-decision-strategy>`. |
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.
this needs a line break before :ref:
Also, the referencenames (book-security-voters-changng-the-access-dicision-strategy
and components-security-authorization-access-decision-manager
) do not exists. I would also prefer to use shorter names, e.g. security-voters-change-strategy
and components-security-access-decision-manager
. For more info about references: https://gist.github.com/WouterJ/5276618
I updated both links with a prepending line break and changed the links to @wouterj suggestion. |
You are still missing the pointers. Sphinx doesn't know anything about the references now. And no, there are no really nice docs about our syntax. We are using Sphinx, an extension of reStructured Text. Both contain only hard to read spefications and references. The only good way to learn is to learn it 'on the fly' |
Do I got it now? :-) |
Yes, 👍 . If you know how to do it, could you please squash the commits? |
from http://symfony.com/doc/current/components/security/authorization.html#access-decision-manager to http://symfony.com/doc/current/cookbook/security/voters.html#changing-the-access-decision-strategy added missing crosslink adding missing crosslink from http://symfony.com/doc/current/cookbook/security/voters.html#changing-the-access-decision-strategy to http://symfony.com/doc/current/components/security/authorization.html#access-decision-manager changed note:: to seealso:: changed note:: to seealso:: added full stop changed to labeled link changed link changed link added pointer added pointer
Alright I squashed them into one! |
👍 Your local merge is also part of this PR. But, I think @weaverryan can cherry pick de233b9 and apply it to the |
@xabbuh yeah I tried to include that one, but it's ging to be split up in a bunch of commits, or maybe I am still a retard ;-) |
fix labels introduced in #3120
I experienced a lack of internal crosslinks. Especially when I was faced with the page: http://symfony.com/doc/current/components/security/authorization.html#access-decision-manager
I was not able to find the configuration snippet as fast as it could be. I came from a search on google to the page and as a user I should be able quickly to jump to any place with related content from this point.
On the page http://symfony.com/doc/current/cookbook/security/voters.html#changing-the-access-decision-strategy the description about the different strategies is not good as on the other one as well. Thats another reason why I added a backlink.
PS: Sorry, Iam not sure if the links correct in this way I added them, I tried. I can also change the box 'note' to a 'tip', if that makes more sense.