Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[book] controller ch review, part 1#6349

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
weaverryan merged 2 commits intosymfony:2.3fromtalitakz:book-controller
Mar 17, 2016
Merged

[book] controller ch review, part 1#6349

weaverryan merged 2 commits intosymfony:2.3fromtalitakz:book-controller
Mar 17, 2016

Conversation

@talitakz
Copy link
Contributor

QA
Doc fix?yes
New docs?no
Applies toall
Fixed ticketsx
  • tried to better form the concept of controller methods and controller classes - nothing new was edded just better formed
  • changes in styling, wording, typos

changes in stylingtried to beter form the concept of controller methods and controller classeswording, typos
@weaverryan
Copy link
Member

WOW! I'll review all of your pr's this week!

@talitakz
Copy link
ContributorAuthor

Thank you! I have also Doctrine in Template chapter in store but first this.
I hope the PR are small enough! :)

@weaverryanweaverryan merged commit888e47e intosymfony:2.3Mar 17, 2016
weaverryan added a commit that referenced this pull requestMar 17, 2016
This PR was merged into the 2.3 branch.Discussion----------[book] controller ch review, part 1| Q             | A| ------------- | ---| Doc fix?      | yes| New docs?     | no| Applies to    | all| Fixed tickets | x- tried to better form the concept of controller methods and controller classes - nothing new was edded just better formed- changes in styling, wording, typosCommits-------888e47e added missing ref in routing ch7a3255e controller ch review, part 1
The new controller returns a simple HTML page. To actually view this page
in your browser, you need to create a route, which maps a specific URL path
to the controller:
to the controller::
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 only 2:: right before aphp block (and then we don't need the.. code-block:: php

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Noted!

weaverryan added a commit that referenced this pull requestMar 17, 2016
Making minor tweaks after reading through the changes in those PR's
@weaverryan
Copy link
Member

Hey @paxyknox!

Phew! Ok I just merged everything in and made some changes here: sha:6214eb6. If you see anything weird, let me know or (better) make a new PR. I like most of your changes - and just made additional changes to some of them.

And yes, the PR's were pretty easy to review :). Here's what I can tell you:

  1. Just make 1 PR for 1 chapter (instead of multiple). Because these cover so much, but don't make HUGE changes, I merge them and then review the sections you changed. I merged all 3 PR's at once, and they conflicted just a little with each other. So, just make one :).

  2. Re-wording is always subjective. If you are sure that something should be re-worded, then do it! But if you're not sure if your new wording is a big improvement, skip it: that saves us time reviewing and merging some "sideways" changes (things that are maybe just "slightly" better... or maybe not better at all).

Thanks!

weaverryan added a commit that referenced this pull requestMar 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  ...
@talitakztalitakz deleted the book-controller branchMarch 18, 2016 10:26
@talitakz
Copy link
ContributorAuthor

I reviewed the merge it looks perfect! :)

Thank you for this instructions on how to make better PR, I really appreciate it!

xabbuh added a commit that referenced this pull requestMar 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 requestMar 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.rstbook/testing.rst
xabbuh added a commit that referenced this pull requestMar 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 freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@talitakz@weaverryan

[8]ページ先頭

©2009-2025 Movatter.jp