Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#6 | 2450383-Fast404ExceptionHtmlSubscriber-DefaultExceptionSubscriber-6.patch | 1.35 KB | joshi.rohit100 |
Comments
Comment #1
martin107 CreditAttribution: martin107 commentedFound another similar error in the class
\Drupal\Core\EventSubscriber\DefaultExceptionSubscriber
is also incorrect the "Html" from the name needs to be dropped.
Comment #2
kushalxp CreditAttribution: kushalxp commentedI didn't you. which file are you talking about? I am new to drupal can you please explain little bit more?
Comment #3
martin107 CreditAttribution: martin107 commentedHi kushalxp
Drupal insists that every class is stored in a file with the same name
so if you use a unix type
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.
Comment #4
joshi.rohit100So should it be separate issue for DefaultExceptionSubscriber or just incoporate in this as it is just one liner ?
Comment #5
martin107 CreditAttribution: martin107 commentedThat 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."
Comment #6
joshi.rohit100Comment #7
joshi.rohit100Both constructor annotation incoporate in this patch.
Comment #8
martin107 CreditAttribution: martin107 commentedCool, Patch looks perfect.
Comment #9
jhodgdonThanks for the patch!
Comment #11
webchickCommitted and pushed to 8.0.x. Thanks!