Skip to content
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

Overhaul documentation #535

Merged
merged 2 commits into from
Mar 13, 2019
Merged

Overhaul documentation #535

merged 2 commits into from
Mar 13, 2019

Conversation

malarzm
Copy link
Member

@malarzm malarzm commented Feb 20, 2019

Currently the bundle's documentation is long and hard to navigate. This PR aims to split it and bring it up to date with Symfony 3.4+ and its standards

I'll gladly accept help with these :)

@malarzm malarzm added this to the 4.0.0 milestone Feb 20, 2019

For more information, see Doctrine's `Mapping Types documentation`_.

.. index::
Copy link
Member Author

Choose a reason for hiding this comment

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

No clue what this does, removed :)

Copy link
Member

Choose a reason for hiding this comment

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

Pinging @javiereguiluz - maybe they know?

Choose a reason for hiding this comment

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

This information is used to generate the "analytical index". Here you can see it: https://symfony.com/doc/current/genindex.html We don't publicize it because we're thinking of removing it and future doc parsers may not support it. It's very useful to create indexes for paper books, where you can display the page where some concept is explained.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the quick explanation! Would you recommend keeping it or is it ok if we drop this?

@malarzm
Copy link
Member Author

malarzm commented Feb 22, 2019

@alcaeus @jmikola on second thought I think that a complete overhaul will not be good for you to review. For example right now I'm tweaking code examples and that'll be impossible to spot basing on the diff only. What do you think about first merging the split and DocumentType note that is currently done and patching things up later?

@malarzm malarzm marked this pull request as ready for review February 22, 2019 20:33
@alcaeus
Copy link
Member

alcaeus commented Feb 25, 2019

Sounds good to me 👍

@alcaeus alcaeus self-requested a review February 25, 2019 05:32
Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

Great work splitting this up - it should be easier for newcomers to get started with ODM this way without reading through tens of pages of docs.

I've highlighted some issues we should address. I'll let you decide which issues you want to fix in this PR and which ones you want to extract to separate issues 👍

Resources/doc/events.rst Outdated Show resolved Hide resolved
Resources/doc/events.rst Outdated Show resolved Hide resolved
Resources/doc/first_steps.rst Outdated Show resolved Hide resolved
Resources/doc/first_steps.rst Outdated Show resolved Hide resolved
Resources/doc/first_steps.rst Outdated Show resolved Hide resolved
Resources/doc/first_steps.rst Outdated Show resolved Hide resolved
Resources/doc/first_steps.rst Outdated Show resolved Hide resolved
Resources/doc/installtion.rst Outdated Show resolved Hide resolved
Resources/doc/installtion.rst Outdated Show resolved Hide resolved
Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@malarzm
Copy link
Member Author

malarzm commented Mar 12, 2019

@javiereguiluz do you know when docs for bundles are built? Or is there maybe a way for you to trigger the build on demand? I'd like to merge this PR but also I'm a bit afraid I'll break something. Perfect for me would be to see the built docs after the built and fixing (and maybe asking you for a rebuild) shortly after not to leave docs broken for a longer time.

@javiereguiluz
Copy link

@malarzm it's an automatic process, so I can't force a doc build for a bundle. The build is done several times per day (I don't know the exact details) so the new docs should be published soon after the merge.

Copy link

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

Very nice work! I reviewed it and left some minor comments.

Resources/doc/events.rst Outdated Show resolved Hide resolved

.. note::

Of course, you can also issue complex queries, which you'll learn more

Choose a reason for hiding this comment

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

In Symfony Docs we have removed words like "Of course", "simply", "obviously", etc. because they are hostile to newcomers. Maybe you want to do the same here and remove "of course".

Copy link
Member Author

@malarzm malarzm Mar 13, 2019

Choose a reason for hiding this comment

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

I'll create separate issue to go through all docs, don't want to do too many changes here as they're hard to spot. Edit: created #539

return $this->redirectToRoute('homepage');
}

Updating an object involves just three steps:

Choose a reason for hiding this comment

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

Most of the times, "just" is another of those bad words for newcomers. If you remove it from this phrase, no meaning is lost and it's more friendly.


.. code-block:: bash

composer config extra.symfony.allow-contrib true

Choose a reason for hiding this comment

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

Not sure if this is needed anymore because Flex now asks you if you want to install a contrib package (always, just this time, only for this vendor, etc.)

Resources/doc/mongodb_library.rst Outdated Show resolved Hide resolved
Resources/doc/security_bundle.rst Outdated Show resolved Hide resolved
@malarzm
Copy link
Member Author

malarzm commented Mar 13, 2019

@javiereguiluz thanks for the reply and review! I'll merge this now and hopefully it builds while I'm creating issues to take care of :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not mention legacy driver Documentation: Document the additional form field types
3 participants