Problem/Motivation

Plugin types and plugins themselves can now use PHP attributes:

Proposed resolution

Add PHP attributes to all plugin types.
Add PHP attributes to all plugin implementations.
Update documentation.

Remaining tasks

Update the documentation

User interface changes

None.

API changes

n/a

Data model changes

n/a

CommentFileSizeAuthor
#13 metatag-n3398796-13.interdiff.txt7.03 KBdamienmckenna
#13 metatag-n3398796-13.patch386.68 KBdamienmckenna
#11 metatag-n3398796-11.patch386.73 KBdamienmckenna
#11 metatag-n3398796-11.interdiff.txt7.07 KBdamienmckenna
#7 metatag-n3398796-7.patch383.86 KBdamienmckenna
#6 metatag-n3398796-6.patch383.97 KBdamienmckenna
#3 metatag-n3398796-3.patch4.13 KBdamienmckenna

Issue fork metatag-3398796

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

DamienMcKenna created an issue. See original summary.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

This involves an API change, so it's going to require a new API and deprecating the old one.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Needs review
StatusFileSize
new metatag-n3398796-3.patch4.13 KB

WIP.

We'll need to deprecate the old APIs and finish updating all modules to the new APIs.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Related issues: +#3252386: Use PHP attributes instead of doctrine annotations
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new metatag-n3398796-6.patch383.97 KB

Further WIP.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new metatag-n3398796-7.patch383.86 KB

Misc syntax fixes.

Status: Needs review » Needs work

The last submitted patch, 7: metatag-n3398796-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #3423884: Plan for Metatag 2.1.0

Need to get a move on so this can be added to 2.1.0.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: Unassigned » damienmckenna

Working on this.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: damienmckenna » Unassigned
Status: Needs work » Needs review
StatusFileSize
new metatag-n3398796-11.interdiff.txt7.07 KB
new metatag-n3398796-11.patch386.73 KB

WIP. It still isn't loading all of the plugins.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Priority: Normal » Major

This is a priority.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new metatag-n3398796-13.patch386.68 KB
new metatag-n3398796-13.interdiff.txt7.03 KB

Typoos.

Status: Needs review » Needs work

The last submitted patch, 13: metatag-n3398796-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: #3423884: Plan for Metatag 2.1.0 »

Pulling this off the schedule until it's closer to being done.

daniel_j’s picture

daniel_j commented
Title: Add PHP annotations for all plugins » Add PHP attributes for all plugins
Version: 2.0.x-dev » 2.2.x-dev

daniel_j opened merge request !224

daniel_j’s picture

daniel_j commented
Issue summary: View changes
Status: Needs work » Needs review

There are a number of phpstan warnings which are unrelated to this issue, mostly having to do with migrating Hooks away from procedural calls. There is also one phpunit failure, which is likewise unrelated (the fb_pages plugin is failing a Javascript test because the configured hostname is invalid).

I think this one's ready to at least have a look at. Documentation has not been updated yet, but it is probably best to do that once the code is likely to remain stable.

daniel_j’s picture

daniel_j commented

It should be noted that I skipped any Tags that (in my opinion) should be deprecated. This includes any IE-specific tags as well as Pocket, which, alas, has gone to the big bitbucket in the sky.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work

A quick thought: how about we split this into two tasks - first convert the group plugins as there are only a few of them, then in a separate issue convert the tag plugins. We also need to retain backwards compatibility for now and drop the old ones in Metatag 3.

daniel_j’s picture

daniel_j commented

As this MR stands, it is 100% backwards-compatible — it supports both Annotations and Attributes. Backwards-compatibility will be broken when the Annotation classes (and their reference in the plugin managers) are removed.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Thank you, that's much appreciated.

Now to fix the test failures.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

The "previous major" test fails, i.e. the tests fail on Drupal 10.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

No, "previous major" is D9. Which is ok to fail.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

This will need to be refactored due to #3574819: Metatag: Convert hooks to OOP going in first.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

The tests fail on Drupal 10, which is a problem.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: Unassigned » damienmckenna

Going to reroll this into a new MR.

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

damienmckenna opened merge request !240

damienmckenna closed merge request !224

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

This fails dramatically on Drupal 10.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

The module fails on 10.5.x with this error:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "metatag_defaults" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (line 142 of core/lib/Drupal/Core/Entity/EntityTypeManager.php).

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

ConfigEntityType was changed to a PHP attribute in 11.1: https://www.drupal.org/node/3505422

As a result this change needs to go into 2.3.0, which would then require core 11.1 and would have to drop Drupal 10 entirely.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

We could revert the MetatagDefaults entity change and get the rest of these changes into 2.2.x..

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review

We'll convert MetatagDefaults to a PHP attribute in a separate issue for Metatag 2.3.0: #3582879: Convert MetatagDefaults config entity to a PHP annotation

  • damienmckenna committed 989fa387 on 2.2.x 
    Issue #3398796 by damienmckenna, daniel_j: Add PHP attributes for all...
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Fixed
Parent issue: » #3548158: Plan for Metatag 2.2.1

Committed. Woot!

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.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: damienmckenna » Unassigned

Status: Fixed » Closed (fixed)

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