Just noticed a glitch in the first line of the annotation above the _construct() function

* Constructs a new CustomPageExceptionHtmlSubscriber.

Should of course relate to the class Fast404ExceptionHtmlSubscriber

There is a similar mistake in DefaultExceptionSubscriber.php

Just leaving this as low hanging fruit ... a nice gentle starter patch for someone.

I will be around to answer any questions.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin107’s picture

Found another similar error in the class

\Drupal\Core\EventSubscriber\DefaultExceptionSubscriber

* Constructs a new DefaultExceptionHtmlSubscriber.

is also incorrect the "Html" from the name needs to be dropped.

kushalxp’s picture

I didn't you. which file are you talking about? I am new to drupal can you please explain little bit more?

martin107’s picture

Hi kushalxp

Drupal insists that every class is stored in a file with the same name

so if you use a unix type

 find . -name Fast404ExceptionHtmlSubscriber.php

or search through the drupal directories in a way that suits you.

the file you want is

./core/lib/Drupal/Core/EventSubscriber/Fast404ExceptionHtmlSubscriber.php

There is documentation which you might find useful

https://www.drupal.org/node/707484 for creating a new patch

I found signing up for the drupal ladder something useful thing to do.

http://drupalladder.org/

I will answer any question here no matter how small.

but in general for other issues there is a also core mentoring https://www.drupal.org/core-office-hours

where you can find someone to help you in irc.

joshi.rohit100’s picture

So should it be separate issue for DefaultExceptionSubscriber or just incoporate in this as it is just one liner ?

martin107’s picture

That is a possibility, If you want the experience of creating a new issue, then I will review both..

But here is my perspective

I have been part of issues, where the complexity of the change is much greater. and I have seen even inserting a single newline get rejected...as out of scope.

If a reviewers train of thought is broken, that can be trouble, but I think that does not apply here.

Depending on which way you go, can, could you fix an error of mine

in comment #1 I should certainly have changed the title of this issue to something more appropriate say

"2 constructor descriptions titles are incorrect."

joshi.rohit100’s picture

Status: Active » Needs review
FileSize
1.35 KB
joshi.rohit100’s picture

Both constructor annotation incoporate in this patch.

martin107’s picture

Title: Fast404ExceptionHtmlSubscriber constructor annotation error » Two incorrectly titled Subscriber constructor functions.
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Cool, Patch looks perfect.

jhodgdon’s picture

Thanks for the patch!

  • webchick committed 98fac0d on 8.0.x
    Issue #2450383 by joshi.rohit100, martin107: Two incorrectly titled...
webchick’s picture

Title: Two incorrectly titled Subscriber constructor functions. » Two incorrectly titled Subscriber constructor functions in docs.
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.