There are some phpcs bugs in 2.2.x. Fix them and make phpcs required in .gitlab-ci.yml.

Issue fork metatag-3616963

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

damienmckenna created an issue. See original summary.

yusuf_khan made their first commit to this issue’s fork.

yusuf_khan’s picture

Status: Active » Needs review

@damienmckenna
Please review

damienmckenna’s picture

Status: Needs review » Needs work

I'd prefer to keep the autowired service and use another mechanism to add it, e.g. extending ServiceProviderBase to add the service if the Migrate module is installed.

I commented out the MigrateHooks class in metatag.services.yml and tried this:


namespace Drupal\metatag;

use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\DependencyInjection\ServiceProviderBase;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;

/**
 * Service Provider for Metatag.
 */
class MetatagServiceProvider extends ServiceProviderBase {

  /**
   * {@inheritdoc}
   */
  public function register(ContainerBuilder $container) {
    $modules = $container->getParameter('container.modules');

    // If the Migrate module is installed, add the 
    if (isset($modules['migrate']) ) {
      // Add a service
      $service_definition = new Definition('Drupal\metatag\Hook\MigrateHooks');
      $service_definition->setClass('Drupal\metatag\Hook\MigrateHooks');
      $service_definition->setAutowired(TRUE);
      $container->setDefinition('Drupal\metatag\Hook\MigrateHooks', $service_definition);
    }
  }

}

With that change it then gives the following error:

Symfony\Component\DependencyInjection\Exception\RuntimeException: Cannot autowire service "Drupal\metatag\Hook\OutputHooks": argument "$metatagManager" of method "__construct()" references class "Drupal\metatag\MetatagManager" but no such service exists. You should maybe alias this class to the existing "metatag.manager" service.

That doesn't make sense to me?

csakiistvan’s picture

@damienmckenna ❌ Does not fix the issue — MR !258 at dbc2275b breaks the container. Answering your question from #5 below.

The MigrateHooks service is not registered by metatag.services.yml but by core's HookCollectorPass, so commenting the YAML entry out and adding a service provider both have no effect. Using #[Autowire(service: 'plugin.manager.migrate.destination')] in the constructor builds the container with Migrate installed and uninstalled alike — both were tried.

damienmckenna changed the visibility of the branch 2.2.x to hidden.

damienmckenna’s picture

damienmckenna’s picture

Status: Needs work » Needs review

damienmckenna changed the visibility of the branch 3616963-fix-phpcs-issues to hidden.

damienmckenna’s picture

I created a clean MR using the two original commits, let's see if it works this time.

  • damienmckenna committed b6b05909 on 2.2.x
    Issue #3616963 by yusuf_khan, DamienMcKenna: Fix phpcs issues.
    
damienmckenna’s picture

Status: Needs review » Fixed

Committed. Thank you!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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