in the file :- core/lib/Drupal/Core/CoreServiceProvider.php, Wrong param Documented.

Here are the changes. 

@@ -105,7 +105,7 @@ public function register(ContainerBuilder $container) {
   /**
    * Determines and registers the UUID service.
    *
-   * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
+   * @param \Drupal\Core\DependencyInjection\ContainerBuilder $container
    *   The container.
    *
    * @return string
@@ -131,6 +131,9 @@ public static function registerUuid(ContainerBuilder $container) {
 
   /**
    * Registers services and event subscribers for a site under test.
+   *
+   * @param \Drupal\Core\DependencyInjection\ContainerBuilder $container
+   *   The container.
    */
   protected function registerTest(ContainerBuilder $container) {
     // Do nothing if we are not in a test environment.
CommentFileSizeAuthor
#4 2606344-4.patch1.03 KBaneeshthankachan
wrong-param-CoreServiceProvider-1.patch977 bytesrakesh.gectcr

Comments

rakesh.gectcr created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
commented

What is inside the Drupal container builder which requires that specific code to be there?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work
Issue tags: +rc eligible 
+++ b/core/lib/Drupal/Core/CoreServiceProvider.php
@@ -105,7 +105,7 @@ public function register(ContainerBuilder $container) {
    *   The container.

@@ -131,6 +131,9 @@ public static function registerUuid(ContainerBuilder $container) {
+   *   The container.

So both of these are documented to be of type ContainerBuilder, but the documentation line says "the container"... it doesn't seem like the right documentation line.

I do not think I understand comment #2?

aneeshthankachan’s picture

aneeshthankachan commented
Status: Needs work » Needs review
StatusFileSize
new 2606344-4.patch1.03 KB

@jhodgdon; I have updated the The container. to The container builder.

dawehner’s picture

dawehner
Primary language German
commented

I do not think I understand comment #2?

The comment #2 was basically about this line:

+++ b/core/lib/Drupal/Core/CoreServiceProvider.php
@@ -105,8 +105,8 @@ public function register(ContainerBuilder $container) {
-   * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
...
+   * @param \Drupal\Core\DependencyInjection\ContainerBuilder $container

What is inside the Drupal container builder which is needed here?

cilefen’s picture

cilefen commented

Expanding on #5, there is no reason to type the parameter as the Drupal implementation when the Symfony one would do. If \Drupal\Core\DependencyInjection\ContainerBuilder has expanded the interface in a way that is needed in this class then it should be the type used in the comment.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -rc eligible

Thanks for the patch! Sorry for delay in review -- I've been on vacation.

So both of these methods have the parameter typed in the function declaration as the Drupal container builder class. Given that, the @param changes in the docs are correct, since they agree with the code. If you want to change the code's declaration of the function (or the use statement at the top of the file), that would be a non-Documentation issue. But for this issue, I think the changes are correct.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed this docs change to both 8.x branches. If you'd like to change the code, please file an issue in a non-documentation component. Thanks all!

  • jhodgdon committed 7449351 on 8.0.x 
    Issue #2606344 by rakesh.gectcr, aneeshthankachan, dawehner, cilefen:...

  • jhodgdon committed 20a4022 on 
    Issue #2606344 by rakesh.gectcr, aneeshthankachan, dawehner, cilefen:...

Status: Fixed » Closed (fixed)

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