clean up for Core/DependencyInjection CompilerPasses

spacing and line breaks were not consistent

Based on the OP and the tags, this sounds like a cleanup task

we dont actually make the code consistant... we make it match standards. :) there is a little difference. For example, if a long time ago, we didn't use newlines to separate code, but now we do. and we are fixing a bug in a file, then in our fix, we use the newlines according to the standard, and do not make it consistant with the rest of the file (which might not be standard).

adding documentation since this will touch/clean up documentation.

Also, if we separate out the documentation cleanups form any code at all, it might be more likely to get in, and we can do the code changes in a separate issue.
But we can post your whole patch here at least and get some feedback on how to do it.

--
using the issue summary template, and a few of the contributor links from (https://docs.google.com/document/d/1xnL18eC2Cwo2YnB0PwmBnn86Eo8aF9CF4qDL...)
--

I'm not sure this is major.
https://drupal.org/node/314328 might help decide.

and I think it's a task.

maybe this is base system?

oh, forgot to link to similar issue. done that now.

Thanks for the patch! A lot of these cleanups are great, especially the ones where we clearly copied the variable names.

1. +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/ModifyServiceDefinitionsPass.php
   @@ -35,5 +35,4 @@ public function process(ContainerBuilder $container) {
      }
   -
    }
   
   +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterBreadcrumbBuilderPass.php
   @@ -29,5 +29,4 @@ public function process(ContainerBuilder $container) {
      }
   -
    }
   

   It's in our coding standard to leave a blank line after each method, including at the end of the class (here and throughout the patch)
   https://drupal.org/node/608152#indenting

2. +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAuthenticationPass.php
   @@ -24,17 +24,17 @@ class RegisterAuthenticationPass implements CompilerPassInterface {
       * @see \Drupal\Core\Authentication\AuthenticationProviderInterface
   +   *
   +   * {@inheritdoc}
       */
      public function process(ContainerBuilder $container) {
   
   +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterRouteEnhancersPass.php
   @@ -19,14 +19,12 @@ class RegisterRouteEnhancersPass implements CompilerPassInterface {
      /**
       * Adds services tagged with "route_enhancer" to the router.
       *
   -   * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
   -   *   The container to process.
   +   * {@inheritdoc}
   
   +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterTwigExtensionsPass.php
   @@ -12,25 +12,22 @@
      /**
       * Adds services tagged 'twig.extension' to the Twig service container.
       *
   -   * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
   -   *   The container to process.
   +   * {@inheritdoc}
       */
   

   Unfortunately we can't add @inheritdoc like this, it either has to stand alone or not be there at all
   https://drupal.org/node/1354#inheritdoc

3. +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAuthenticationPass.php
   @@ -24,17 +24,17 @@ class RegisterAuthenticationPass implements CompilerPassInterface {
   -    // Get the authentication manager.
   ...
   -    // Iterate all autentication providers and add them to the manager.
   

   Why remove these lines?

4. +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterKernelListenersPass.php
   @@ -20,7 +24,7 @@ public function process(ContainerBuilder $container) {
   -      // We must assume that the class value has been correcly filled, even if the service is created by a factory
   +      // We must assume that the class value has been correctly filled, even if the service is created by a factory
   

   This should be wrapped to 80 char and end with a period

5. +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterTwigExtensionsPass.php
   @@ -12,25 +12,22 @@
        }
   -
        $definition = $container->getDefinition('twig');
   

   Generally leaving a blank line after these early-exit checks is better for readability

+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterKernelListenersPass.php
@@ -2,7 +2,7 @@
+ * Contains of Drupal\Core\DependencyInjection\Compiler\RegisterKernelListenersPass.

This should probably be just "Contains", not "Contains of"

#6.2 yeah.
some people want to change that: #1994890: Allow {@inheritdoc} and additional documentation.
but for now, it is all or nothing.

[edit: tim had a good idea in irc. just writing it here so it doesn't get lost. "consider moving them to inline docs in the methods themselves" So, use inheritdoc, and instead of just deleting the docs that are adding information, also move that info into the methods as inline comments. :) ]

#6.4 yeah.
Lines containing comments (including docblocks) must wrap as close to 80 characters as possible without going over, with a few exceptions (noted in the Tag Reference below).
https://drupal.org/node/1354#drupal

#7 yeah.
https://drupal.org/node/1354#file

next steps,
another patch.

also include an interdiff please. the interdiff instructions might help:
https://drupal.org/documentation/git/interdiff

oh, and when you upload next patch, change the issue status to needs review to send it to the bot, and signal people following this that you are looking for feedback.

@cordoval thanks for the interdiff posted in comment #9.
to keep the text bot from testing interdiffs, name them with a .txt ending (the testbot tests anything ending in .patch)
(by definition interdiffs will not apply to head, they should apply to head with the previous patch applied. :) )

in the interdiff doc: https://drupal.org/documentation/git/interdiff it is easy to miss that it should be .txt

You can upload more than one file at a time, usually we upload the interdiff and patch at the same time.

-------
So I think the file in #9 was the interdiff between the patch in #5 and the patch in #11. :)

-------
let's leave the instructions for how to make a patch and how to do a review in the issue summary until this is committed. :) We will probably need to do those things a few times more yet.

Note: Most of the changes here are deprecated by #2213319: Create a single Container CompilerPass to collect + add handlers to consumer service definitions

+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterKernelListenersPass.php
@@ -2,7 +2,7 @@
+ * Contains Drupal\Core\DependencyInjection\Compiler\RegisterKernelListenersPass.

Afaik we do use a leading slash here.

Here's a straight reroll.

Another straight reroll. :-)

This patch is getting smaller and smaller.

+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php
@@ -16,9 +16,7 @@
-   * Adds services tagged 'access_check' to the access_manager service.

Can we move down the documentation into the function body at least?

Added the documentation inside the method body.

Those are all fair small cleanups! Thanks a ton

Committed/pushed to 8.1.x and 8.2.x thanks!

Correcting issue status, see #2698635: Issue statuses changing by themselves?.