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.

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

Complete review of the "Customize Error Pages" cookbook article #5117

Merged
merged 7 commits into from
May 23, 2015

Conversation

javiereguiluz
Copy link
Member

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

In my opinion the current article looks "old" compared to other parts of the documentation. I've reworded it completely to make it more actionable and to explain more clearly the three alternatives provided to developers.


Thus, error pages can be customized in different ways, depending on how
much control you need:
.. image:: /images/cookbook/controller/error_pages/exceptions-in-dev-environment.png
Copy link
Member

Choose a reason for hiding this comment

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

You have to remove the controller/ part of the path.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Actually I think the error was that images should be under controller/error_pages/. I've moved the image files.

a configurable (but otherwise arbitrary) controller to generate the
response. The default controller used has a sensible way of
picking one out of the available set of error templates.
In the `development environment`_ Symfony catches all the exceptions and displays
Copy link
Member

Choose a reason for hiding this comment

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

"In the development environment, Symfony catches [...]"

Thus, error pages can be customized in different ways, depending on how
much control you need:
.. image:: /images/cookbook/controller/error_pages/exceptions-in-dev-environment.png
:alt: A typical exception page in development environment
Copy link
Member

Choose a reason for hiding this comment

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

[...] in the development [...]

@javiereguiluz
Copy link
Member Author

I've added all the suggested changes. @xabbuh please review that "the labels for the old and removed headings" are OK because I'm not sure what I'm doing.

#. :ref:`Replace the default exception controller with your own
(intermediate). <custom-exception-controller>`
.. image:: /images/cookbook/controller/error_pages/errors-in-prod-environment.png
:alt: A typical error page in the production environment
Copy link
Member

Choose a reason for hiding this comment

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

maybe add a tip that you can see the production error page in the dev env by clicking on the link?

Copy link
Member

Choose a reason for hiding this comment

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

what link can you click to see the production error page?

Copy link
Member

Choose a reason for hiding this comment

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

I thought you added that feature?

Copy link
Member

Choose a reason for hiding this comment

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

I had a PR for that, but then my PR was replaced with a whole other, better proposal for 2.6 (which I don't believe has any link like that).

``Resources/views/Exception`` directory of the TwigBundle to render the error
page. If you browse that directory (usually located in
``vendor/symfony/symfony/src/Symfony/Bundle/TwigBundle``) you'll find a lot of
templates defined for different types of errors and content formats.
Copy link
Member

Choose a reason for hiding this comment

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

maybe also mention that error.*.twig is used in prod and exception.*.twig in dev? (if I remember correctly)

@javiereguiluz
Copy link
Member Author

@wouterj I've addressed most of your comments and suggestions. Could we please label this important PR as "finished" and merge it? Thanks!

@weaverryan weaverryan merged commit faad683 into symfony:2.3 May 23, 2015
weaverryan added a commit that referenced this pull request May 23, 2015
… article (javiereguiluz)

This PR was merged into the 2.3 branch.

Discussion
----------

Complete review of the "Customize Error Pages" cookbook article

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

In my opinion the current article looks "old" compared to other parts of the documentation. I've reworded it completely to make it more actionable and to explain more clearly the three alternatives provided to developers.

Commits
-------

faad683 Address most of the comments made by Wouter
3842b61 Changes suggested by reviewers
3073783 Rewordings
722b9d0 Fixed the location of the cookbook article images
e304a9d Fixed a restructuredtext syntax issue
5ea54bf Fixed a reStructuredText formatting issue
b54a050 Complete review of the "Customize Error Pages" cookbook article
@weaverryan
Copy link
Member

Thanks Javier!

Sorry for the slow merge - this kind of update is SO great. I merged it and made some more tweaks and shortenings in #5304.

Cheers!

weaverryan added a commit that referenced this pull request May 23, 2015
… shortening some things (weaverryan)

This PR was merged into the 2.3 branch.

Discussion
----------

Proofreading Javier's excellent updates - in some places, shortening some things

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.3+
| Fixed tickets | #5117

Hi guys!

After @javiereguiluz's excellent updates in #5117, I proofread the first section (the most important one) even more. The big takeaway is that I tried to shorten some things - less talking, more showing (the original version was quite "long", Javier shortened some things, I'm proposing going even further).

Thanks!

Commits
-------

5dc0f36 Fixes thanks to the team
ef97575 Proofreading Javier's excellent updates - in some places, shortening things even more
@javiereguiluz javiereguiluz deleted the update_error_customization branch January 3, 2018 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants