Problem/Motivation

I think that the documentation comment for the $classResolver protected variable in MainContentViewSubscriber may be incorrect.

Steps to reproduce

Look at line 30 in Drupal/Core/EventSubscriber/MainContentViewSubscriber.php, where $classResolver is documented as \Drupal\Core\Controller\ControllerResolverInterface whereas I think maybe it should be Drupal\Core\DependencyInjection\ClassResolverInterface.

Proposed resolution

Update that documentation comment.

Remaining tasks

  • Confirm whether it is true that this change should be made.
  • Make the change.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

CommentFileSizeAuthor
#7 3174570-3.patch634 bytesquietone
#5 After-patch-document.png60.93 KBMadhu kumar
#4 3174570-applied_patch.png21.13 KBabhijith s
#3 3174570-3.patch634 bytesanmolgoyal74

Comments

skudderk created an issue. See original summary.

cilefen’s picture

cilefen commented
Issue tags: +Documentation
anmolgoyal74’s picture

anmolgoyal74 commented
Status: Active » Needs review
StatusFileSize
new 3174570-3.patch634 bytes
abhijith s’s picture

abhijith s commented
StatusFileSize
new 3174570-applied_patch.png21.13 KB

Applied patch #3 and it works fine.The documentation error is fixed in this patch.

after

RTBC +1

Madhu kumar’s picture

Madhu kumar commented
StatusFileSize
new After-patch-document.png60.93 KB

Patch #3 Applied cleanly and working as Expected , sharing screenshot for reference.

quietone’s picture

quietone commented
Version: 8.9.x-dev » 9.3.x-dev
Status: Needs review » Reviewed & tested by the community
Issue tags: +Bug Smash Initiative

The patch still applies to 9.3.x and the change looks correct to me.

@Abhijith S and Madhu kumar - thank you for looking into this issue. Posting screenshots of your codebase or CLI does not advance the issue, So, I've removed the issue credit for that screenshot. In the future, you can get credit for issues by reading the issue to understand its purpose, and posting your review or testing of that purpose. Thank you!

quietone’s picture

quietone commented
StatusFileSize
new 3174570-3.patch634 bytes

Arg, the automatic testing is testing with 8.9.x not 9.3.x. I am re-uploading the patch to hopefully fix that.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: 3174570-3.patch, failed testing. View results

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Reviewed & tested by the community

Back to RTBC per #6 after #3255836: Test fails due to Composer 2.2 solved the unrelated test failure.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 9.4.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 5296e875c6d to 10.0.x and ca7158d4612 to 9.4.x and e537bf66d7c to 9.3.x. Thanks!

  • alexpott committed 5296e87 on 10.0.x 
    Issue #3174570 by quietone, anmolgoyal74, skudderk: Fix documentation...

  • alexpott committed ca7158d on 9.4.x 
    Issue #3174570 by quietone, anmolgoyal74, skudderk: Fix documentation...

  • alexpott committed e537bf6 on 9.3.x 
    Issue #3174570 by quietone, anmolgoyal74, skudderk: Fix documentation...

Status: Fixed » Closed (fixed)

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