Change record: Plugin implementations should use PHP attributes instead of annotations

I already had this implemented in #3523808: Fix failing pipelines but then decided to split it into a separate issue so it can be more thoroughly reviewed and tested.

Issue fork search_api-3539549

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

drunken monkey created an issue. See original summary.

drunken monkey opened merge request !254

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review

Opened an MR with the code from #3523808: Fix failing pipelines.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

This merge request looks great and I think we can merge this. Since the discovery of normal annotations is not yet disabled, this shouldn't impact any plugins out in the wild.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

Thanks for the feedback, good to hear!
I think I’ll still leave it open for a few days more before merging, just to be on the safe side.

I also now stumbled over #3099471: Deprecate "hidden" definition key in favor of "no_ui", which I had forgotten but makes sense – all other of our plugin types now have a $no_ui property, but processors only have $hidden, which we now had to make explicit. (Up to now we had silently accepted both.)
The good thing about this move is that people will have to manually (or semi-manually) switch from annotations to attributes anyways, so now would probably be a perfect moment to switch to $no_ui, if we want. Or to add both but deprecate $hidden right away.
Not sure whether the additional consistency is worth the hassle, though. Any opinions on this?

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

I think it's a good idea to do, but if we can create a rector rule, to automate the refactoring, it doesn't have to happen at the same time.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

You’re probably right, no need to do both at the same time. Thanks!
Merged.

  • drunken monkey committed a69c3114 on 8.x-1.x 
    Issue #3539549 by drunken monkey, borisson_: Added attribute discovery...

Status: Fixed » Closed (fixed)

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