Skip to content
Snippets Groups Projects

Throw deprecations when a plugin type with attributes is using annotations

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • added 1 commit

    • b05be94b - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • added 48 commits

    • b05be94b...aac47f0d - 37 commits from branch project:11.x
    • 87a4062a - 1 earlier commit
    • 70517513 - use correct error type
    • 47b21266 - Add working test coverage
    • 51379c89 - update deprecation message and start expecting it in legacy test
    • ba29c394 - Improve deprecation message to include the plugin id
    • b389facc - Remove test_xss_title
    • c1bca0c3 - Dynamically define missing trait and interface
    • 8c9f2552 - Avoid CS errors
    • 95dea23a - remove deprecation unit test coverage
    • e8abc868 - convert more block plugins
    • a2897b21 - adjust block ui test for removed xss block plugin

    Compare with previous version

  • added 697 commits

    • a2897b21...8756d339 - 686 commits from branch project:11.x
    • 61157b16 - 1 earlier commit
    • 9671ac83 - use correct error type
    • 515cf104 - Add working test coverage
    • 568ad1ad - update deprecation message and start expecting it in legacy test
    • c67a13e7 - Improve deprecation message to include the plugin id
    • f6a37668 - Remove test_xss_title
    • 9532ad66 - Dynamically define missing trait and interface
    • ff55ce34 - Avoid CS errors
    • beaeacad - remove deprecation unit test coverage
    • 8ec74988 - convert more block plugins
    • aebde2e2 - adjust block ui test for removed xss block plugin

    Compare with previous version

  • Lee Rowlands
  • Looks great, agree we need to action this asap

  • added 202 commits

    • aebde2e2...8f28c48d - 191 commits from branch project:11.x
    • 7b6bcf39 - 1 earlier commit
    • bb81bcb3 - use correct error type
    • cafa62d2 - Add working test coverage
    • 2aae046f - update deprecation message and start expecting it in legacy test
    • c296cd51 - Improve deprecation message to include the plugin id
    • b06966a7 - Remove test_xss_title
    • d57fab4f - Dynamically define missing trait and interface
    • 63280a11 - Avoid CS errors
    • 38be23c4 - remove deprecation unit test coverage
    • 9a82aaac - convert more block plugins
    • faa54bbc - adjust block ui test for removed xss block plugin

    Compare with previous version

  • added 57 commits

    • faa54bbc...fc0aa24e - 46 commits from branch project:11.x
    • fb9f7366 - 1 earlier commit
    • 4b65bd44 - use correct error type
    • f647caec - Add working test coverage
    • 9a2dd508 - update deprecation message and start expecting it in legacy test
    • d6d1fe9d - Improve deprecation message to include the plugin id
    • 89561f49 - Remove test_xss_title
    • cb5a70a5 - Dynamically define missing trait and interface
    • 4f010576 - Avoid CS errors
    • b1758207 - remove deprecation unit test coverage
    • 315f9081 - convert more block plugins
    • 44553cba - adjust block ui test for removed xss block plugin

    Compare with previous version

  • added 33 commits

    • 44553cba...181221a4 - 22 commits from branch project:11.x
    • 55a9f981 - 1 earlier commit
    • a7b63484 - use correct error type
    • 3d802436 - Add working test coverage
    • df1358ef - update deprecation message and start expecting it in legacy test
    • cb531d00 - Improve deprecation message to include the plugin id
    • 68620b9f - Remove test_xss_title
    • 0dd8ea3e - Dynamically define missing trait and interface
    • 7ffd0d55 - Avoid CS errors
    • aebeb5a8 - remove deprecation unit test coverage
    • c9d29fea - convert more block plugins
    • b0b463c3 - adjust block ui test for removed xss block plugin

    Compare with previous version

  • added 1065 commits

    Compare with previous version

  • added 1 commit

    • 7e5e817b - make phpstan ignore BC traits

    Compare with previous version

  • added 1 commit

    • 61d7d4f1 - different phpstan workaround, full ignore for that file

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    • a853c3c2 - fix media mapping constraint

    Compare with previous version

  • added 1 commit

    • ba5b50ab - convert another new entity type, drop unnecessary stuff

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • Lee Rowlands
  • Looking great, thanks for picking this up

  • added 464 commits

    Compare with previous version

  • added 1 commit

    • 4d682741 - Deprecate annotation only discovery

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • quietone
  • added 1 commit

    Compare with previous version

  • catch left review comments

    left review comments

  • added 331 commits

    Compare with previous version

  • added 16 commits

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • godotislate
  • added 1 commit

    • 51664f0b - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • Derek Wright resolved all threads

    resolved all threads

  • Derek Wright left review comments

    left review comments

  • Derek Wright
    Derek Wright @dww started a thread on the diff
  • 88 88 $parser = new StaticReflectionParser($class, $finder, TRUE);
    89 89
    90 90 $reflection_class = $parser->getReflectionClass();
    91 // @todo Handle deprecating definitions discovery via annotations in
    92 // https://www.drupal.org/project/drupal/issues/3265945.
    93 91 /** @var \Drupal\Component\Annotation\AnnotationInterface $annotation */
    94 92 if ($annotation = $this->getAnnotationReader()->getClassAnnotation($reflection_class, $this->pluginDefinitionAnnotationName)) {
    95 93 $this->prepareAnnotationDefinition($annotation, $class);
    96 return ['id' => $annotation->getId(), 'content' => $annotation->get()];
    94
    95 $id = $annotation->getId();
    96 $shortened_annotation_name = '@' . substr($this->pluginDefinitionAnnotationName, strrpos($this->pluginDefinitionAnnotationName, '\\') + 1);
    97 // phpcs:ignore
    98 @trigger_error(sprintf('Using %s annotation for plugin with ID %s is deprecated and is removed from drupal:13.0.0. Use a %s attribute instead. See https://www.drupal.org/node/3395575', $shortened_annotation_name, $id, $this->pluginDefinitionAttributeName), E_USER_DEPRECATED);
    • Aren't we supposed to say when it started being deprecated here?

      Suggested change
      98 @trigger_error(sprintf('Using %s annotation for plugin with ID %s is deprecated and is removed from drupal:13.0.0. Use a %s attribute instead. See https://www.drupal.org/node/3395575', $shortened_annotation_name, $id, $this->pluginDefinitionAttributeName), E_USER_DEPRECATED);
      98 @trigger_error(sprintf('Using %s annotation for plugin with ID %s is deprecated in drupal:11.2.0 and is removed from drupal:13.0.0. Use a %s attribute instead. See https://www.drupal.org/node/3395575', $shortened_annotation_name, $id, $this->pluginDefinitionAttributeName), E_USER_DEPRECATED);
    • Explained in the issue summary, we do not include this.

    • I read the issue summary but disagree that this message should not include the version when the deprecation happened. I think we should not be vague, this should be as detailed as possible. Even though the ability was added in 10.2, it is only now, in 11.2, that core is announcing that it is deprecated.

    • @quietone the problem is that while all core plugins provide an attribute now, there are also contrib plugins to worry about, and it would not be accurate to say that it's possible to define those using attributes from 11.2, because in many cases it won't be until the contrib modules support attributes themselves. So I don't really think we have a choice here.

    • Please register or sign in to reply
  • Derek Wright
  • Derek Wright
  • Derek Wright
  • Derek Wright
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading