Deprecate views_entity_field_label() in favor of a class method outside Views

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

If I'm understanding the deprecation was figured out in D9 so this should be updated to do the deprecation warning.

Will need a test to show the message.

Addressing 13 we don't need tests showing deprecation messages per the deprecation policy.

This looks good to me, the only question is about BC, I think EntityFieldManagerInterface is a 1:1 interface and we can add methods there, but it could still break of someone for example decorates that service.

Implementing the interface finds finds only one contrib, and that's a false positive, that has its own Interface, it also doesn't extend core.
https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22imple...

extends finds a few, two test stubs and one real subclass, those _should_ be fine:
https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22exten...

Who knows what kind of shenanigans people did with custom code :shrug:

Was about to RTBC, but the change record should have a before/after example even if it's trivial. It possibly should also more clearly say that this method has been added to EntityFieldManagerInterface.

I updated the CR thanks!

Looks good to me now.

This needs a rebase and I left some comments in the MR.

@nicxvan, why did you tag this as 11.2.0 release priority?

> @nicxvan, why did you tag this as 11.2.0 release priority?

Because this is a blocker to deprecate hook_hook_info() usage, as it relies on those extra include files being loaded.

Great thanks! I applied most suggestions, I'll rebase and address the rest of your comments later today or early tomorrow.

@berdir is right this is one of two blockers to deprecate hook hook info which is the real target.

Ok I address your comments and rebased, I fixed one more typo as well.

I'll keep an eye on tests and move status after.

added more more comment that I missed before.

Rebased and addressed feedback

Thanks.

Added some suggestions for very minor / pedantic nits. Otherwise, seems reasonable to me.

Tests are green after applying nits.

Thanks, looks better.

Issue title and summary look clear and accurate.

Saving credit for quietone and myself for MR reviews.

Don’t see anything else to improve. RTBC++

This looks good but needs a rebase.

Working on this.

Rebased thanks!

phpstan is failing in the pipeline.

Related, but the rebase seems to have clobbered views.views.inc entirely. I thought we still needed a deprecated views_entity_field_label() in there.

TL;DR: Probably wise not to self-RTBC after a rebase, especially with merge conflicts, so that peer review can help spot trouble. 😅

Yeah I'm working on baseline, the deprecated function was moved to views.module

Sorry, I missed something in previous reviews (or this is a new bug). 1 suggestion to apply, otherwise seems RTBC to me.

p.s. I did some minor edits and formatting cleanups on the CR: https://www.drupal.org/node/3489411/revisions/view/13800107/13851975 →

• Pipeline is now green.
• All feedback addressed.
• All threads resolved.
• Title and summary are accurate and clear.
• CR is simple, accurate and clear.

I see nothing left to improve. RTBC.

Thanks!
-Derek

Committed/pushed to 11.x, thanks!

catch → closed merge request !10309

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