doc: Review Getting Started #2650

GromNaN · 2024-06-21T11:01:42Z

Q	A
Type	doc
BC Break	no
Fixed issues	-

GromNaN · 2024-06-21T11:02:13Z

docs/en/tutorials/getting-started.rst

@@ -263,12 +279,15 @@ If you want to iterate over the posts the user references it is as easy as the f

    <?php

-    $posts = $dm->getPosts();


That was an error. $user->getPosts() expected.

GromNaN · 2024-06-21T11:03:12Z

docs/en/tutorials/getting-started.rst

@@ -199,44 +230,29 @@ Here is how you would use your models now:
    Note that you do not need to explicitly call persist on the ``$post`` because the operation
    will cascade on to the reference automatically.

-Now if you did everything correctly, you should have those two objects
-stored in MongoDB in correct collections and databases. You can use the
-`php-mongodb-admin project, hosted on github`_ to look at your


This was a PHP UI for MongoDB, but archived. Replaced with Compass.

GromNaN · 2024-06-21T11:03:45Z

docs/en/tutorials/getting-started.rst


-    $dm = DocumentManager::create(null, $config);
+    $dm = DocumentManager::create(config: $config);


Using named arguments here. More expressive.

GromNaN · 2024-06-21T11:05:17Z

docs/en/tutorials/getting-started.rst

+You will notice that working with objects is nothing magical and you only have
+access to the properties and methods that you have defined yourself so the
+semantics are very clear. You can continue reading about the MongoDB in the
+:doc:`Introduction to MongoDB Object Document Mapper <../reference/introduction>`.


I find this page very similar to the introduction. It shows how to instantiate the DocumentManager and create classes using mapping.

Noted. That sounds like something worth revisiting after the docs are cleared up. Ultimately, it may be something our docs team can assist with, as they did for Laravel.

alcaeus

Small nit in the example, but looking good other than that 👍

docs/en/tutorials/getting-started.rst

GromNaN · 2024-06-21T13:55:40Z

docs/en/tutorials/getting-started.rst

+        public function __construct(
+            public string $name = '',
+            public string $email = '',
+            public array $posts = [],


I think the modeling is done the wrong direction. It's the post that should refer to the user, not the user that contains the list of posts.

I agree, although this may have been intentional for a "Getting Started" document so that inverse relationships could be introduced at a later point: https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/2.7/reference/bidirectional-references.html#owning-and-inverse-sides

docs/en/tutorials/getting-started.rst

jmikola · 2024-06-21T14:25:24Z

docs/en/tutorials/getting-started.rst

+        public function __construct(
+            public string $name = '',
+            public string $email = '',
+            public array $posts = [],


I agree, although this may have been intentional for a "Getting Started" document so that inverse relationships could be introduced at a later point: https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/2.7/reference/bidirectional-references.html#owning-and-inverse-sides

jmikola · 2024-06-21T14:27:54Z

docs/en/tutorials/getting-started.rst

 Persistent Models
 -----------------

-To make the above classes persistent, all we need to do is provide Doctrine with some mapping
-information so that it knows how to consume the objects and persist them to the database.
+To make the above classes persistent, all we need to do is provide Doctrine with


Suggested change

To make the above classes persistent, all we need to do is provide Doctrine with

To make the above classes persistent, we need to provide Doctrine with

MongoDB's own docs have a style guide to avoid subjective language like this. It also comes across as filler, so I have no qualms about removing it.

@GromNaN: I think "all we need to do" can still be removed.

docs/en/tutorials/getting-started.rst

jmikola · 2024-06-21T14:30:56Z

docs/en/tutorials/getting-started.rst

-`php-mongodb-admin project, hosted on github`_ to look at your
-``BlogPost`` collection, where you will see only one document:
+After running this code, you should have those two objects stored in MongoDB in
+the collections "User" and "BlogPost". You can use the `MongoDB Compass`_


Suggested change

the collections "User" and "BlogPost". You can use the `MongoDB Compass`_

the collections "User" and "BlogPost". You can use `MongoDB Compass`_

Unless you were writing "the MongoDB Compass tool", you can omit "the" when referring to it as a proper noun.

jmikola · 2024-06-21T15:08:06Z

docs/en/tutorials/getting-started.rst

-stored in MongoDB in correct collections and databases. You can use the
-`php-mongodb-admin project, hosted on github`_ to look at your
-``BlogPost`` collection, where you will see only one document:
+After running this code, you should have those two objects stored in MongoDB in


stored in the "User" and "BlogPost" collections.

I think we can omit "in MongoDB" to avoid repetition of "in".

Also, would the collection names end up being users and blogposts? I forget whether ODM uses inflection to create the namespaces for each model.

jmikola · 2024-06-21T15:10:18Z

docs/en/tutorials/getting-started.rst

@@ -263,12 +279,15 @@ If you want to iterate over the posts the user references it is as easy as the f

    <?php

-    $posts = $dm->getPosts();
-    foreach ($posts as $post) {
+    foreach ($user->posts as $post) {


This is where I'd expect posts to be a PersistentCollection. It may be too much information to mention that here (or earlier when you use a Collection interface), though, so I won't advocate for a note.

Adding a note that explains the behavior of PersistentCollection.

jmikola · 2024-06-21T15:13:46Z

docs/en/tutorials/getting-started.rst

-reading about the MongoDB in the :doc:`Introduction to MongoDB Object Document Mapper <../reference/introduction>`.
+You will notice that working with objects is nothing magical and you only have
+access to the properties and methods that you have defined yourself so the
+semantics are very clear. You can continue reading about the MongoDB in the


"so the semantics are very clear" can probably be removed, as it makes the sentence run on a bit.

I'd also advocate for removing "the MongoDB" and maybe just rewriting the second sentence entirely:

You can continue reading [LINK]

jmikola · 2024-06-21T15:14:43Z

docs/en/tutorials/getting-started.rst

+You will notice that working with objects is nothing magical and you only have
+access to the properties and methods that you have defined yourself so the
+semantics are very clear. You can continue reading about the MongoDB in the
+:doc:`Introduction to MongoDB Object Document Mapper <../reference/introduction>`.


Noted. That sounds like something worth revisiting after the docs are cleared up. Ultimately, it may be something our docs team can assist with, as they did for Laravel.

jmikola

LGTM but one of my earlier suggestions was overlooked.

jmikola · 2024-06-22T02:01:21Z

docs/en/tutorials/getting-started.rst

 Persistent Models
 -----------------

-To make the above classes persistent, all we need to do is provide Doctrine with some mapping
-information so that it knows how to consume the objects and persist them to the database.
+To make the above classes persistent, all we need to do is provide Doctrine with


@GromNaN: I think "all we need to do" can still be removed.

jmikola · 2024-06-22T02:02:57Z

docs/en/tutorials/getting-started.rst

@@ -138,6 +162,13 @@ You can provide your mapping information in Annotations or XML:
          </document>
        </doctrine-mongo-mapping>

+.. note::
+
+   The `$id` property is a special property that is used to store the unique


Nit: technically, the property mapped with #[ODM\Id] is special. The property name itself can be anything. If you want to address this, I'd say something like:

The $id property above is annotated with #[ODM\Id], which makes it special...

Here, I'm using "annotated" as a verb. My word choice has nothing to do with attributes vs. annotations :)

* 2.9.x: (24 commits) Fix typo in code example (#2670) Label PRs about GH actions with "CI" (#2632) Review basic mapping (#2668) Fix wording (#2667) Add native type to private properties and final classes (#2666) Review and add tests on `ResolveTargetDocumentListener` (#2660) Remove soft-delete-cookbook (#2657) doc: Remove wakeup and clone cookbook (#2663) Modernize generated code for Hydrators (#2665) Add tests for introduction (#2664) doc: Review mapping ORM and ODM cookbook (#2658) doc: Review cookbook on blending ORM and ODM (#2656) doc: Review and test validation cookbook (#2662) Update custom mapping example (#2654) doc: Review Simple Search Engine Cookbook (#2659) doc: Add cookbook about embedding referenced documents using $lookup (#2655) doc: Add type to properties (#2652) doc: Review custom collections and repository docs (#2653) doc: Review Getting Started (#2650) Move annotations-reference to attributes-reference (#2651) ...

GromNaN commented Jun 21, 2024

View reviewed changes

GromNaN marked this pull request as ready for review June 21, 2024 11:05

GromNaN requested a review from alcaeus June 21, 2024 11:05

alcaeus approved these changes Jun 21, 2024

View reviewed changes

docs/en/tutorials/getting-started.rst Outdated Show resolved Hide resolved

GromNaN force-pushed the doc-getting-started branch from 92d82cf to 62d73b6 Compare June 21, 2024 13:19

GromNaN commented Jun 21, 2024

View reviewed changes

jmikola reviewed Jun 21, 2024

View reviewed changes

GromNaN force-pushed the doc-getting-started branch from 9da83a3 to 373d565 Compare June 21, 2024 18:55

GromNaN requested a review from jmikola June 21, 2024 18:55

SenseException approved these changes Jun 21, 2024

View reviewed changes

jmikola approved these changes Jun 22, 2024

View reviewed changes

GromNaN added 4 commits June 24, 2024 15:16

Review Getting Started

be3c693

Fix typo

b007bf8

Review fixes

b47ff60

wording

bb2beac

GromNaN force-pushed the doc-getting-started branch from 373d565 to bb2beac Compare June 24, 2024 13:20

GromNaN merged commit aeee4a4 into doctrine:2.9.x Jun 24, 2024
17 of 18 checks passed

GromNaN deleted the doc-getting-started branch June 24, 2024 13:21

GromNaN added the Documentation label Jun 26, 2024

GromNaN added this to the 2.9.0 milestone Jun 27, 2024

github-actions bot mentioned this pull request Sep 20, 2024

Merge release 2.9.0 into 3.0.x #2677

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: Review Getting Started #2650

doc: Review Getting Started #2650

GromNaN commented Jun 21, 2024

GromNaN Jun 21, 2024

GromNaN Jun 21, 2024 •

edited

Loading

GromNaN Jun 21, 2024

GromNaN Jun 21, 2024

jmikola Jun 21, 2024

alcaeus left a comment

GromNaN Jun 21, 2024

jmikola Jun 21, 2024

jmikola Jun 21, 2024

jmikola Jun 21, 2024

jmikola Jun 22, 2024

jmikola Jun 21, 2024

jmikola Jun 21, 2024

jmikola Jun 21, 2024

GromNaN Jun 21, 2024

jmikola Jun 21, 2024

jmikola Jun 21, 2024

jmikola left a comment

jmikola Jun 22, 2024

jmikola Jun 22, 2024

		@@ -263,12 +279,15 @@ If you want to iterate over the posts the user references it is as easy as the f

		<?php

		$posts = $dm->getPosts();


		$dm = DocumentManager::create(null, $config);
		$dm = DocumentManager::create(config: $config);

	To make the above classes persistent, all we need to do is provide Doctrine with
	To make the above classes persistent, we need to provide Doctrine with

	the collections "User" and "BlogPost". You can use the `MongoDB Compass`_
	the collections "User" and "BlogPost". You can use `MongoDB Compass`_

doc: Review Getting Started #2650

doc: Review Getting Started #2650

Conversation

GromNaN commented Jun 21, 2024

Choose a reason for hiding this comment

GromNaN Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alcaeus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmikola left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GromNaN Jun 21, 2024 •

edited

Loading