Skip to content

DOCSP-35892: Quick Start docs #2721

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

Merged
merged 23 commits into from
Feb 8, 2024

Conversation

ccho-mongodb
Copy link
Contributor

@ccho-mongodb ccho-mongodb commented Feb 5, 2024

Quick Start Tutorial

JIRA: https://jira.mongodb.org/browse/DOCSP-35892

Staging:
Quick Start
Driver Landing Page Summary

Note to reviewer:
The download repo is still in progress and will be captured in another ticket.

Checklist

  • Add tests and ensure they pass
  • Add an entry to the CHANGELOG.md file
  • Update documentation for new features

@ccho-mongodb ccho-mongodb marked this pull request as draft February 5, 2024 18:27
Copy link
Contributor

@norareidy norareidy left a comment

Choose a reason for hiding this comment

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

Looks good, just some wording suggestions / questions!

@ccho-mongodb ccho-mongodb changed the title Docsp 35892 quick start docs DOCSP-35892: Quick Start docs Feb 6, 2024
Copy link
Contributor

@norareidy norareidy left a comment

Choose a reason for hiding this comment

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

LGTM with some final suggestions!

.. procedure::
:style: connected

.. step:: Set the Connection String in the Database Configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

I: per the style guide, I think these steps should use sentence-style capitalization

Copy link
Contributor

Choose a reason for hiding this comment

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

Applies to the other steps/files too

Comment on lines 34 to 37
Proceed to the :guilabel:`Connect your application` section and select
"PHP" from the :guilabel:`Driver` selection menu and the version
that best matches the version you installed from the :guilabel:`Version`
selection menu.
Copy link
Contributor

Choose a reason for hiding this comment

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

S: this sentence is a little long, think it can be broken up

Suggested change
Proceed to the :guilabel:`Connect your application` section and select
"PHP" from the :guilabel:`Driver` selection menu and the version
that best matches the version you installed from the :guilabel:`Version`
selection menu.
Proceed to the :guilabel:`Connect your application` section. Select
"PHP" from the :guilabel:`Driver` selection menu and the version
that best matches the version you installed from the :guilabel:`Version`
selection menu.

Comment on lines 28 to 33
.. note::

If you run into issues on this step, ask for help in the
:community-forum:`MongoDB Community Forums <>`
or submit feedback by using the :guilabel:`Share Feedback`
tab on the right or bottom right side of this page.
Copy link
Contributor

Choose a reason for hiding this comment

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

S: replace this note with the troubleshoot.rst file

Suggested change
.. note::
If you run into issues on this step, ask for help in the
:community-forum:`MongoDB Community Forums <>`
or submit feedback by using the :guilabel:`Share Feedback`
tab on the right or bottom right side of this page.
.. include:: /includes/quick-start/troubleshoot.rst

Comment on lines 19 to 21
Update the ``store()`` method ``MovieController.php``, located in
the ``app/Http/Controllers`` directory to insert request data
into the database as shown in the following code:
Copy link
Contributor

Choose a reason for hiding this comment

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

S: I think the current wording is slighly unclear, something like this might work:

Suggested change
Update the ``store()`` method ``MovieController.php``, located in
the ``app/Http/Controllers`` directory to insert request data
into the database as shown in the following code:
Navigate to the ``MovieController.php`` file located in
the ``app/Http/Controllers`` directory. Then, update the ``store()`` method to insert request data
into the database as shown in the following code:

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 can see how "navigate to" might be a step in an IDE in which you need to expand folder contents, but on the command line, there's no need to change directory to open a file for editing, so I chose to stay away from that wording.

Which part was unclear, and do you think this could be a good alternative (omits describing what they're accomplishing and focuses only on the actions they need to take to proceed):

"Replace the store() method in the MovieController.php file, located in the app/Http/Controllers" directory with the following code:"

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay that makes sense. The main issue I had was the "update the store() method MovieController.php" part of the sentence since it seemed incomplete (but rewording to "Replace the store() method in the MovieController.php file" fixes that issue). I also think there's a missing comma after "directory," but otherwise the alternative wording you gave works for me!


.. step:: View the Data

Open ``http://127.0.0.1:8000/browse_movies`` in your web browser to view
Copy link
Contributor

Choose a reason for hiding this comment

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

S: nit, but since this is a URL and it's clickable in view-data.txt, could be helpful to also make this link clickable:

Suggested change
Open ``http://127.0.0.1:8000/browse_movies`` in your web browser to view
Open http://127.0.0.1:8000/browse_movies in your web browser to view


New to Laravel? Check out our bootcamp and documentation. Build something amazing!

.. step:: Add a Laravel Application Encryption Key
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: do you need to run this command in the my-app directory? If so, I'd mention that in the step

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good callout! I will move the "cd" command into this step (currently in the next step) to make it clear.

@ccho-mongodb ccho-mongodb marked this pull request as ready for review February 7, 2024 19:36
.gitignore Outdated
@@ -9,3 +9,4 @@ composer.phar
phpunit.xml
phpstan.neon
/.cache/
docs/.vscode
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a configuration that should be global to your system instead of specific to every project you work on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the link! I'll remove this line from the .gitignore.

.. code-block:: json
:copyable: false

"mongodb/laravel-mongodb": "^{+package-version+}"
Copy link
Member

Choose a reason for hiding this comment

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

It's good that you put the caret prefix to allow new minor versions.

@ccho-mongodb ccho-mongodb merged commit e5634ac into mongodb:4.1 Feb 8, 2024
@ccho-mongodb ccho-mongodb deleted the DOCSP-35892-quick-start-docs branch February 8, 2024 15:41
@GromNaN GromNaN added the docs label Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants