-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Cleaned up documentation #734
Conversation
Cleaned up documentation, tweaked some of the grammar, changed class#method() references to class::method(), added warning about performance impact of lazy loading
Hello, thank you for creating this pull request. I have automatically opened an issue http://www.doctrine-project.org/jira/browse/DDC-2571 We use Jira to track the state of pull requests and the versions they got |
@@ -395,8 +394,8 @@ new script for this: | |||
echo sprintf("-%s\n", $product->getName()); | |||
} | |||
|
|||
The ``EntityManager#getRepository()`` method can create a finder object (called | |||
repository) for every entity. It is provided by Doctrine and contains some | |||
The ``EntityManager::getRepository()`` method can create a finder object (called |
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.
We actually use #
for non-static calls. ::
is used only for static method calls (yes, using ::
for both is a bad practice of the php community)
I don't think the page is consistent... it uses I put this question on stackexchange and the general suggestion from the community was to avoid using # (since it's not a generally recognised standard) and just stick to :: in all cases. |
@caponica agreed on what the community does, not agreeing that the community does it right :) |
Well, if we're going to use # for non-static calls we should at least be consistent ourselves! I'll update the whole doc to change all the relevant documentation references from :: to # when I get a chance and then update the PR. |
@caponica agreed, and thanks :) |
Do you also want to use this notation for class properties, e.g. should it be |
@caponica not sure to be honest. @guilhermeblanco ? |
(by the way, I have to agree with the peeps on stackexchange that using a # is really weird in a PHP context, but I'll go with whatever house style you prefer!) |
A couple of other comments about this page of the documentation are below. Either I'm not bright enough to understand what's happening in them all, or they seem to be inconsistent with the code displayed. The following paragraph is quite opaque and does not explain itself very well:
I cannot see where the column names have been changed/set:
This paragraph seems confusing, since the join definition is simply
|
First paragraph should be like this:
|
Not sure about the second paragraph (where is it located?) Third paragraph looks like a personal thought. It could be manipulated to state that an additional number of properties and settings can be added to the many-to-many mapping |
Hi, you can view the current version of this here: http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/tutorials/getting-started.html I'll work on tidying up the first paragraph. The second paragraph appears after the following block of XML code, which I don't think has any specially defined column names (it just uses the defaults as far as I can see):
The third paragraph is confusing (me!) since it talks about the complexity of defining the products relationship, but that relationship is actually just a simple many-to-many join. Maybe the paragraph should just be removed? |
Replaced ``class::$field`` with ``class#field`` to match Doctrine style Cleaned up three paragraphs mentioned in doctrine#734
OK, I've cleaned up the references to the class properties to use the Entity#name style (instead of Entity::$name) and rewritten the three paragraphs that I think were a bit confusing. I think this is now ready to merge in but let me know if there's anything else to fix! |
Thank you for this! |
Replaced ``class::$field`` with ``class#field`` to match Doctrine style Cleaned up three paragraphs mentioned in doctrine#734
Cleaned up documentation, tweaked some of the grammar, edits for improved readability, changed class#method() references to class::method(), added warning about performance impact of lazy loading