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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor update to the Error Handling docs #181

Open
wants to merge 4 commits into
base: 3.20.x
Choose a base branch
from

Conversation

settermjd
Copy link
Contributor

Q A
Documentation yes
Bugfix no
BC Break no
New Feature no
RFC no
QA no

Description

This change:

  • Completes some of the PHP code examples, so that it's clear that they refer to an entire file, not just a part of a file
  • Provides an example for enabling the example LoggingErrorListenerDelegatorFactory used in the docs, for those unfamiliar with delegator factories (and those who are a little rusty with them 馃槃).

@settermjd settermjd self-assigned this Mar 15, 2024
This change completes some of the PHP code examples, so that it's clear
that the example refers to an entire file, as well as provides an
example for enabling the example LoggingErrorListenerDelegatorFactory
used in the docs, for those unfamiliar with delegator factories (and
those who are a little rusty with them).

Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
@@ -195,6 +200,9 @@ You could then use a [delegator factory](container/delegator-factories.md) to
create your logger listener and attach it to your error handler:

```php
<?php
Copy link
Member

Choose a reason for hiding this comment

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

IMO just noise, but I don't know our convention here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could well be and I appreciate what you're saying. However, it's a convention that I've seen in most other online documentation and examples wrt PHP for some time now.

Copy link
Member

Choose a reason for hiding this comment

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

@settermjd
The problem here is that we don't have a definition to mark parts of code from files.

See in the documentation of Doctrine ORM:

This example is logical:

<?php
// src/Bug.php

class Bug
{
    public function close()
    {
        $this->status = "CLOSE";
    }
}

https://www.doctrine-project.org/projects/doctrine-orm/en/current/tutorials/getting-started.html#updating-entities

But this one is not:

<?php
$product = $entityManager->getRepository('Product')
                         ->findOneBy(array('name' => $productName));

https://www.doctrine-project.org/projects/doctrine-orm/en/current/tutorials/getting-started.html#entity-repositories


Therefore, any suggestion is welcome. Please add it to the documentation repository.
Thanks in advance! 馃憤馃徎

@@ -154,6 +156,9 @@ allowing the listener the ability to introspect the generated response as well.
As an example, you could create a logging listener as follows:

```php
<?php
Copy link
Member

Choose a reason for hiding this comment

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

IMO just noise, but I don't know our convention here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As above, mate.

docs/book/v3/features/error-handling.md Outdated Show resolved Hide resolved
Copy link
Member

@froschdesign froschdesign left a comment

Choose a reason for hiding this comment

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

Nice improvement! 馃憤馃徎
I have added some comments.

docs/book/v3/features/error-handling.md Outdated Show resolved Hide resolved
Comment on lines +159 to +161
<?php
declare(strict_types=1);

Copy link
Member

Choose a reason for hiding this comment

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

Excerpt from the Writers Guide:

To standardize the examples and reduce unnecessary repetition, the following rules must be observed:

  • PHP's open and close tags are only allowed in code examples for view scripts
  • the declare construct must be omitted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might it be worth adding a documented exception in this case? As always, I'm happy to go with the group consensus.

Comment on lines +203 to +205
<?php
declare(strict_types=1);

Copy link
Member

Choose a reason for hiding this comment

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

Excerpt from the Writers Guide:

To standardize the examples and reduce unnecessary repetition, the following rules must be observed:

  • PHP's open and close tags are only allowed in code examples for view scripts
  • the declare construct must be omitted

docs/book/v3/features/error-handling.md Outdated Show resolved Hide resolved
docs/book/v3/features/error-handling.md Outdated Show resolved Hide resolved
settermjd and others added 3 commits March 20, 2024 11:29
Co-authored-by: Frank Br眉ckner <info@froschdesignstudio.de>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Co-authored-by: Frank Br眉ckner <info@froschdesignstudio.de>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Co-authored-by: Frank Br眉ckner <info@froschdesignstudio.de>
Signed-off-by: Matthew Setter <matthew@matthewsetter.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants