- Issue created by @drunken monkey
- 🇦🇹Austria drunken monkey Vienna, Austria
Let’s see whether just adding the attribute works with all Drupal versions. Otherwise, we’ll probably have to just ignore those errors for now, until we depend on the relevant Drupal Core version.
- 🇦🇹Austria drunken monkey Vienna, Austria
The Postgres fail looks like it’s just a CI problem (database not running), nothing to do with us. I’ll run the pipeline again.
- 🇦🇹Austria drunken monkey Vienna, Austria
OK, this seems to work and be backwards-compatible, just like the change record → promised.
Now we just need to go through around 40 other plugins and make this change.
Anyone interested in a simple, if slightly annoying, issue credit? Otherwise I’ll try to get to it when I have more time (and might just silence those deprecation warnings in the meantime).Regarding the Postgres failure: unfortunately that seems to be persistent. Does anyone have any idea what could be going wrong there?
In the schedule config, I’m setting_TARGET_DB_VERSION
to$CORE_PGSQL_MIN
– is that maybe wrong? - 🇬🇧United Kingdom scott_euser
Spacing is off and can't just be fixed with phpcbf, but should be much quicker
To get this for future travellers:
- Take raw output from the gitlab pipeline and save as pipeline-output.txt
grep '^ Using @' pipeline-output.txt > only-annotation-deprecations.txt
sed -E -n "s/^[[:space:]]*Using @([^ ]+) .*Use a (.*) attribute instead\..*$/new \\\DrupalRector\\\Drupal10\\\Rector\\\ValueObject\\\AnnotationToAttributeConfiguration('10.0.0', '10.0.0', '\1', '\2'),/p" only-annotation-deprecations.txt > rector-ready.txt
- (install drupal rector)
- (add output from the above to drupal rector)
vendor/bin/rector -c rector.php process modules/contrib/search_api
- 🇬🇧United Kingdom scott_euser
Mostly working, though not sure about that failing test whether related or not...
- 🇦🇹Austria drunken monkey Vienna, Austria
Thanks a lot for your work on this, much appreciated!
According to this change record → the Views plugin types were converted to attribute discovery in Drupal 10.3 so I think we need to depend on that in order to drop the BC annotations right away. However, since 10.2 is already unsupported (unless I’m mistaken it even seems like support for 10.3 was just dropped) I think this should be fine.
Surprisingly, the failing test is related: Switching our Views row plugin(s) back to use the
@ViewsRow
annotation does fix the test. The functionality tested is implemented insearch_api_search_api_index_insert()
but it seems the call to\Drupal::getContainer()->get('plugin.manager.views.row')->clearCachedDefinitions()
does not work when using an attribute instead of an annotation. Not even sprinkling further such calls throughout the code (e.g., in the test method itself right after creating the new index, or in\Drupal\views_ui\Form\Ajax\Display::__construct()
) fixes the test.
On the other hand, the functionality seems to work fine when trying it out manually (adding a new index and checking whether you can set its row style to “Rendered entity” without a manual cache clear) so this seems to be a tests-only problem, which likely would have to be fixed in Drupal Core.
However, I now spent several hours of precious contrib time on this and don’t have time to file a Core issue with the necessary information and (ideally) failing test case. Leaving this for either anyone else to take care of or for me to later pick up when I have the time. For now, I guess we have to leave the workaround in place to make pipelines pass.The final remaining deprecation warnings regard our own plugin types and their lack of attributes. I added attribute discovery for backend plugins but this also was a bit of work so I had to leave it at that. If noone else currently has the time we can ignore those warnings for now and add a follow-up issue.