Problem/Motivation

In #3252386: Use PHP attributes instead of doctrine annotations we added support for attribute based plugin discovery.
As part of that issue we converted block and action plugins.

This issue is to convert \Drupal\Core\Annotation\Mail plugins to use Attributes.

Proposed resolution

  1. Add a class to represent the new Attribute - Example
  2. Update the plugin manager constructor to include both the attribute and annotation class names - example
  3. Convert all plugins that use the annotation to use the new attribute - example

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3420977

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:

About issue forks

Comments

larowlan created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: Unassigned » larowlan

Giving one of these a shot to test the instructions

larowlan opened merge request !6565

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: larowlan » Unassigned
Status: Active » Needs review

Looks like the instructions worked - MR https://git.drupalcode.org/project/drupal/-/merge_requests/6565

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Based on how Block and Action are this looks inline with those.

Searched for @Mail and all instances appear to be updated.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs work

larowlan credited Berdir.

larowlan credited mstrelan.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Needs review

Addressed reviews

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Appears all feedback has been addressed, thanks @mstrelan for those threads I'll be keeping an eye out for that kind of stuff as more tickets roll in.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Looks great to me. A new attribute, which is then used in the plugin manager, and a 1:1 replacement of the plugins themselves.

Annotation deprecations have not yet landed but we don't need to wait for that here.

Committed a2c8988 and pushed to 11.x. Thanks!

Will update the relevant change record to include mail plugins.

  • longwave committed a2c8988d on 11.x 
    Issue #3420977 by larowlan, smustgrave, Berdir, mstrelan: Convert Mail...

longwave closed merge request !6565

Status: Fixed » Closed (fixed)

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