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

Fix minor problems in book/page_creation.rst #5635

Closed
wants to merge 2 commits into from
Closed

Fix minor problems in book/page_creation.rst #5635

wants to merge 2 commits into from

Conversation

fabschurt
Copy link

Q A
Doc fix? yes
New docs? no
Applies to >= 2.6
Fixed tickets N/A

the :doc:`form system </book/forms>`, using :doc:`Doctrine </book/doctrine>`
if you need to query a database and more. in the :doc:`Symfony Book </book/index>`.
if you need to query a database and more in the :doc:`Symfony Book </book/index>`.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we enclose "if you need to query database" with commas?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so, it's just a list. However, I find this sentence very hard to read.

Copy link
Author

Choose a reason for hiding this comment

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

@wouterj Yes, me too. Maybe it would be more readable by putting it like that? :

Then, in the Symfony Book, learn about the service container, the form system,
using Doctrine (if you need to query a database), and more!

Copy link
Member

Choose a reason for hiding this comment

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

Big 👍 (except that we don't use serial comma's like the one before "and more!")

Copy link
Author

Choose a reason for hiding this comment

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

Noted 😉 I'm going to modify the PR.

Copy link
Member

Choose a reason for hiding this comment

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

👍 That's indeed much better.

@wouterj
Copy link
Member

wouterj commented Aug 22, 2015

👍

xabbuh added a commit that referenced this pull request Aug 22, 2015
This PR was submitted for the 2.6 branch but it was merged into the 2.3 branch instead (closes #5635).

Discussion
----------

Fix minor problems in book/page_creation.rst

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | >= 2.6
| Fixed tickets | N/A

Commits
-------

5a4a51b Fix minor problems in book/page_creation.rst
@xabbuh
Copy link
Member

xabbuh commented Aug 22, 2015

Thank you @fabschurt. I have merged your patch into the 2.3 branch. That's why it is shown as closed here.

@xabbuh xabbuh closed this Aug 22, 2015
@fabschurt
Copy link
Author

Sure, actually I had branched from 2.6 since the piece of documentation that was modified didn't exist in 2.3, so I was indeed wondering how you were going to resolve that 😅

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.

3 participants