-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[RFR] Misc. minor fixes mostly related to formatting issues #3899
10000 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
Q | A |
---|---|
Doc fix? | yes |
New docs? | no |
Applies to | 2.3+ |
Fixed tickets | - |
@@ -1034,7 +1034,7 @@ Cache Invalidation | |||
------------------ | |||
|
|||
"There are only two hard things in Computer Science: cache invalidation | |||
and naming things." --Phil Karlton | |||
and naming things." — Phil Karlton |
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.
you used an em dash here. There are more places in the docs where the em dash should be used, but we used a hyphen instead there (because @weaverryan found it easier to just use the hyphen: #1637 (comment))
We should decided which dash we use and use that consistently in all docs
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.
You are right. I should always follow the style guide. Reverting this change.
* generate repository classes configured with the | ||
``@ORM\Entity(repositoryClass="...")`` annotation; | ||
* generate repository classes configured with the | ||
``@ORM\Entity(repositoryClass="...")`` annotation; |
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.
Is there any special reason for the additional indentation on this line?
Adding a tip inside a short sidebar isn't very normal. Moreover it doesn't look right on the website.
rendering the documentation on the website
I prefer not to add more commits to this PR to avoid getting too big. If you agree with the changes/fixes, please merge it. |
So far, I think it looks fine. Besides, you could squash your commits if you feel that they get somewhat messy. |
is specific to the :class:`Symfony\\Bundle\\FrameworkBundle\\Controller\\ControllerResolver` | ||
sub-class used by the Symfony2 Framework. | ||
#. The ``AcmeDemoBundle:Default:index`` format of the ``_controller`` key | ||
is changed to another string that contains the full class and method |
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 will change the letters to numbers, did you make sure references to the list items are also updated?
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.
Yes, I did my best to check if letter were reference. I found none.
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.
@javiereguiluz according to @xabbuh in the other thred, the bug was fixed by the indentation change, ot by the switch from a)
to #.
. So you could try switching it back
Did you test all lists that were changed to #. in this PR? That syntax can be buggy sometimes |
@wouterj yes, I checked the three lists. I even checked them with the GitHub renderer and everything seems fine. |
array(), | ||
array(), | ||
array('HTTP_HOST' => 'm.' . $client->getContainer()->getParameter('domain')) | ||
); |
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.
Now, the contents of the code block is no longer indented by four spaces, is it?
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.
The code block was wrongly indented. I "fixed" it ... but I forgot to reindent the code-block
directive. It's fixed now.
The original problem was solved with a proper indentation of the contents, so it's not necessary to change the way the item numbers are defined.
Sweeeeeeeet! |
…es (javiereguiluz) This PR was merged into the 2.3 branch. Discussion ---------- [RFR] Misc. minor fixes mostly related to formatting issues | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | 2.3+ | Fixed tickets | - Commits ------- 2442839 Reverted the changes in the numbered lists d38f00e Fixed a minor indenttation issue 03b83ba Splitted one tip directive into two smaller tips b223417 Replaced a sidebar with a tip directive e871a32 Fixed a very minor indentation issue 3b557a0 Fixed the formatting of several lists to avoid problems when rendering the documentation on the website 2deb5d7 Added some links to API methods 6625ff2 Fixed the indentation of a bulleted list 81fe666 Fixed more table headers formatting 314c131 Fixed the indentation of one code block 9a71577 Removed a tip inside a sidebar 4641c66 Fixed the formatting of one table headers 0554cae Fixed some minor formatting issues related to lists dee3180 Fixed the formating of one table (it now uses proper headers) 00b3d26 Added the suggestions made by the great @wouterj 79bf0a1 Misc. minor fixes mostly related to formatting issues