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

Editing the Doctrine section to improve accuracy and readability #6360

Merged
merged 3 commits into from
Mar 17, 2016

Conversation

natechicago
Copy link
Contributor

By clarifying sections of the documentation that are a little ambiguous or poorly-worded, these changes should make Symfony's introduction to Doctrine more accurate, expressive, and easier to read, particularly for users who have never worked with Doctrine before.

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets n/a

@@ -111,8 +111,8 @@ information. By convention, this information is usually configured in an
of your project, like inside your Apache configuration, for example. For
more information, see :doc:`/cookbook/configuration/external_parameters`.

Now that Doctrine knows about your database, you can have it create the database
for you:
Now that Doctrine can connect to your database server, the following command
Copy link
Member

Choose a reason for hiding this comment

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

I would remove "server" here as Doctrine also supports database platforms which aren't run on a server (e.g. SQLite, which is commonly used)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point, thanks! I've pushed the change.

@@ -738,8 +749,6 @@ Doctrine's native SQL-like language called DQL to make a query for this::
)->setParameter('price', '19.99');

$products = $query->getResult();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed lines 741-742 because the getOneOrNullResult() method is introduced in more detail further down in this document on line 768. Including it here is a little premature/redundant.

@weaverryan weaverryan merged commit 8bfb9e0 into symfony:2.3 Mar 17, 2016
weaverryan added a commit that referenced this pull request Mar 17, 2016
…ability (natechicago)

This PR was merged into the 2.3 branch.

Discussion
----------

Editing the Doctrine section to improve accuracy and readability

By clarifying sections of the documentation that are a little ambiguous or poorly-worded, these changes should make Symfony's introduction to Doctrine more accurate, expressive, and easier to read, particularly for users who have never worked with Doctrine before.

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets | n/a

Commits
-------

8bfb9e0 Editing the Doctrine section to improve readability.
b80cf6d Editing the Doctrine section to improve readability.
2464a67 Editing the Doctrine section to improve readability.
The table name annotation is optional. If it's omitted, Doctrine will
assume that the entity's class name should double as the database table
name. In the example above, an explicit definition was provided to force
the table name to be lowercased.
Copy link
Member

Choose a reason for hiding this comment

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

I actually changed this back - it's a bit more complicated nowadays, because the SE comes with the lowercase strategy (https://github.com/symfony/symfony-standard/blob/master/app/config/config.yml#L59) so the table name does not necessarily equal the entity name.

weaverryan added a commit that referenced this pull request Mar 17, 2016
@weaverryan
Copy link
Member

Thanks @natechicago - these were really nice changes - all the docs need a re-read from time to time (and thanks for not making TOO many changes - it was pretty easy to review). I made some small tweaks at sha: 10d19ba

Cheers!

weaverryan added a commit that referenced this pull request Mar 17, 2016
* 2.3: (24 commits)
  [#6365] Removing extra :
  Added minor clarification
  [#6360] Minor changes
  [#6349][#6351][#6352]
  Editing the Doctrine section to improve readability.
  Minor corrections
  Fixed typo
  Fix escaping of backtick inside double back-quotes
  Removed server:stop code block for 2.3
  Removed the PR table example (this is now included by GitHub template)
  Updated link to Translatable Extension
  [reference] [constraints] added missing colon character for Image constraint documentation in YAML format.
  Editing the Doctrine section to improve readability.
  Removed info about reducing visibility for private
  Updated link to Translatable Extension
  Editing the Doctrine section to improve readability.
  typo
  controller ch review, part 3
  typo
  controller ch review, part 2
  ...
xabbuh added a commit that referenced this pull request Mar 26, 2016
* 2.7: (32 commits)
  Fixed wrong code examples for Isbn constraint
  unused use instructions
  Fix typo in SwitchUserListener file name
  Changed folder name to lowercase (best practises)
  [#6365] Removing extra :
  Add a note about enabling DebugBundle to use VarDumper inside Symfony
  Update introduction.rst
  Added minor clarification
  Changed folder name to lowercase (best practises)
  Fixed typo in path
  [#6360] Minor changes
  [#6349][#6351][#6352]
  Editing the Doctrine section to improve readability.
  Minor corrections
  Fixed typo
  Fix escaping of backtick inside double back-quotes
  Removed server:stop code block for 2.3
  Removed the PR table example (this is now included by GitHub template)
  Updated link to Translatable Extension
  [reference] [constraints] added missing colon character for Image constraint documentation in YAML format.
  ...

Conflicts:
	book/controller.rst
xabbuh added a commit that referenced this pull request Mar 26, 2016
* 2.8: (37 commits)
  Fixed wrong code examples for Isbn constraint
  Calling the parent implementation is mandatory.
  unused use instructions
  Fix typo in SwitchUserListener file name
  Reworded the example about $deep param
  Changed folder name to lowercase (best practises)
  [#6365] Removing extra :
  Add a note about enabling DebugBundle to use VarDumper inside Symfony
  Update introduction.rst
  Added minor clarification
  Changed folder name to lowercase (best practises)
  Fixed typo in path
  [#6360] Minor changes
  [#6349][#6351][#6352]
  Update "bootstrap.php.cache" to "autoload.php"
  Editing the Doctrine section to improve readability.
  Minor corrections
  Fixed typo
  Fix escaping of backtick inside double back-quotes
  Made list of types more consistent
  ...

Conflicts:
	book/installation.rst
	book/testing.rst
xabbuh added a commit that referenced this pull request Mar 26, 2016
* 3.0: (38 commits)
  Fixed wrong code examples for Isbn constraint
  Calling the parent implementation is mandatory.
  unused use instructions
  Fix typo in SwitchUserListener file name
  Reworded the example about $deep param
  Changed folder name to lowercase (best practises)
  [#6365] Removing extra :
  Add a note about enabling DebugBundle to use VarDumper inside Symfony
  Update introduction.rst
  Added minor clarification
  Changed folder name to lowercase (best practises)
  Fixed typo in path
  [#6360] Minor changes
  [#6349][#6351][#6352]
  Update "bootstrap.php.cache" to "autoload.php"
  Editing the Doctrine section to improve readability.
  Minor corrections
  Fixed typo
  Fix escaping of backtick inside double back-quotes
  Made list of types more consistent
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants