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.

Sign up for GitHub

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

Jump to bottom

[quick tour] mostly typos #6275

Closed
wants to merge 4 commits into from
Closed

[quick tour] mostly typos #6275

wants to merge 4 commits into from

Conversation

talitakz
Copy link
Contributor

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets x

javiereguiluz
javiereguiluz reviewed Feb 15, 2016
View reviewed changes
quick_tour/the_architecture.rst
@@ -309,4 +309,4 @@ need to learn a lot to become a Symfony master. Ready to dig into these
topics now? Look no further - go to the official :doc:`/book/index` and
pick any topic you want.

.. _Composer: https://getcomposer.org
.. _`Composer`: https://getcomposer.org
Copy link
Member

Choose a reason for hiding this comment

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

This is technically not wrong ... but I prefer this syntax too :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The proposed changed is more common throughout the documentation.

@xabbuh
Copy link
Member

xabbuh commented Feb 20, 2016

I need to find some time to review this as this is really a lot of changes. On the first glance they look good but I need to do some deeper reviewing. I cannot promise when I will have the time to do the review but please feel free to ping me if I forget about it.

weaverryan
weaverryan reviewed Mar 6, 2016
View reviewed changes
quick_tour/the_big_picture.rst
When you browsed ``http://localhost:8000/app/example`` earlier, Symfony executed
the controller defined in the ``src/AppBundle/Controller/DefaultController.php``
If you'd browsed ``http://localhost:8000/app/example`` for example, Symfony could
executed the controller defined in the ``src/AppBundle/Controller/DefaultController.php``
Copy link
Member

Choose a reason for hiding this comment

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

When you go to ``http://localhost:8000/app/example``, Symfony will execute the
controller in ``src/AppBundle/Controller/DefaultController.php`` and render the ...

@weaverryan
Copy link
Member

👍 once @paxyknox can make the changes mention in the comments. Thanks!

wouterj
wouterj reviewed Mar 8, 2016
View reviewed changes
quick_tour/the_big_picture.rst
@@ -43,7 +44,7 @@ Actions and Controllers
~~~~~~~~~~~~~~~~~~~~~~~

Open the ``src/AppBundle/Controller/DefaultController.php`` file and you'll
see the following code (for now, don't look at the ``@Route`` configuration
see the following code (for now, don't look at the ``@Route()`` configuration
Copy link
Member

Choose a reason for hiding this comment

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

I'm -1 on this change, annotations are like classes. We use the () to indicate functions

@talitakz
Copy link
Contributor Author

I made requested changes.
If there are no other remarks this could probably be merged.

@wouterj
Copy link
Member

wouterj commented Mar 11, 2016

👍 Thanks for the detailed review and fixes @paxyknox!

xabbuh added a commit that referenced this pull request Mar 11, 2016
@xabbuh
minor #6275 [quick tour] mostly typos (Talita Kocjan Zager)
2270e12 
This PR was squashed before being merged into the 2.3 branch (closes #6275).

Discussion
----------

[quick tour] mostly typos

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

Commits
-------

61415c2 [quick tour] mostly typos
xabbuh added a commit that referenced this pull request Mar 11, 2016
@xabbuh
[#6275] some minor tweaks
adfc746
@xabbuh
Copy link
Member

xabbuh commented Mar 11, 2016

Thank you @paxyknox for this very nice review. I have merged your pull request into the 2.3 branch and made some very minor tweaks in adfc746.

@xabbuh xabbuh closed this Mar 11, 2016
xabbuh added a commit that referenced this pull request Mar 11, 2016
@xabbuh
Merge branch '2.3' into 2.7
82ba7db 
* 2.3:
  [#6219] some tweaks
  Point that route parameters are also Request attributes
  [#6348] some minor tweaks
  [best practices] mostly typos
  [#6275] some minor tweaks
  [quick tour] mostly typos
  remove link-local IPv6 address (fe80::1)
  [#6305] move link reference to the bottom
  Mention IvoryCKEditorBundle in the Symfony Forms doc
  [#6328] minor tweak
  Update extension.rst - added caution box for people trying to remove the default file with services definitions
  Altered single / multiple inheritance sentence
  Replace XLIFF number ids by strings
xabbuh added a commit that referenced this pull request Mar 11, 2016
@xabbuh
Merge branch '2.7' into 2.8
e87e9ee 
* 2.7:
  [#6219] some tweaks
  Point that route parameters are also Request attributes
  [#6348] some minor tweaks
  [best practices] mostly typos
  [#6275] some minor tweaks
  [quick tour] mostly typos
  remove link-local IPv6 address (fe80::1)
  [#6305] move link reference to the bottom
  Mention IvoryCKEditorBundle in the Symfony Forms doc
  [#6328] minor tweak
  Update extension.rst - added caution box for people trying to remove the default file with services definitions
  Altered single / multiple inheritance sentence
  Replace XLIFF number ids by strings
  Rename DunglasApiBundle to ApiPlatform
xabbuh added a commit that referenced this pull request Mar 11, 2016
@xabbuh
Merge branch '2.8' into 3.0
f24b8f8 
* 2.8:
  [#6219] some tweaks
  Point that route parameters are also Request attributes
  [#6348] some minor tweaks
  [best practices] mostly typos
  [#6275] some minor tweaks
  [quick tour] mostly typos
  remove link-local IPv6 address (fe80::1)
  [#6305] move link reference to the bottom
  Mention IvoryCKEditorBundle in the Symfony Forms doc
  [#6328] minor tweak
  Update extension.rst - added caution box for people trying to remove the default file with services definitions
  Altered single / multiple inheritance sentence
  Replace XLIFF number ids by strings
  Rename DunglasApiBundle to ApiPlatform
xabbuh added a commit that referenced this pull request Mar 11, 2016
@xabbuh
Merge branch '3.0'
92abae9 
* 3.0:
  [#6219] some tweaks
  Point that route parameters are also Request attributes
  [#6348] some minor tweaks
  [best practices] mostly typos
  Fix reference to app folder
  [#6275] some minor tweaks
  [quick tour] mostly typos
  remove link-local IPv6 address (fe80::1)
  [#6305] move link reference to the bottom
  Mention IvoryCKEditorBundle in the Symfony Forms doc
  [#6328] minor tweak
  Update extension.rst - added caution box for people trying to remove the default file with services definitions
  Altered single / multiple inheritance sentence
  Replace XLIFF number ids by strings
  Rename DunglasApiBundle to ApiPlatform
@talitakz talitakz deleted the quick-tour branch March 18, 2016 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@talitakz @xabbuh @weaverryan @wouterj @javiereguiluz