Problem/Motivation

There are only two remaining functions in field.module, let's move them and deprecate.

Proposed resolution

field_form_field_config_edit_form_entity_builder() -> move to a method on FieldConfigEditForm
It is the only place it is used and contrib does not use it.

_field_create_entity_from_ids() -> move to an internal helper in ContentEntityStorageBase and deprecate it with no replacement.

Remaining tasks

Review.

Issue fork drupal-3572173

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

nicxvan created an issue. See original summary.

ben280398’s picture

ben280398 commented

Working on this

nicxvan’s picture

nicxvan commented
Status: Active » Postponed

This is not ready either sorry. We still need confirmation on the plan.

amateescu made their first commit to this issue’s fork.

amateescu opened merge request !14712

amateescu’s picture

amateescu commented
Issue summary: View changes
Status: Postponed » Needs review

Updated the plan and posted a MR :)

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs review » Needs work

Left some remarks

amateescu’s picture

amateescu commented
Status: Needs work » Needs review

Fixed review points and the tests.

nicxvan’s picture

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

This looks great now, I think all feedback has been addressed.

I did a review over slack on the deprecations and changing the form callbacks allowing DI.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Needs work

Added two suggestions.

nicxvan’s picture

nicxvan commented

Added code suggestions for everything but the deprecation.

They could use a review before I apply them.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Issue tags: +Needs change record
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Assigned: Unassigned » claudiu.cristea

Let's finish this

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Assigned: claudiu.cristea » Unassigned
Status: Needs work » Needs review

So far...

  • Addressed review remarks
  • Updated and linked the CR

Ready for review

nicxvan’s picture

nicxvan commented

One question on the merge request

nicxvan’s picture

nicxvan commented
Status: Needs review » Needs work

There are a couple of open comments.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review
Issue tags: -Needs change record

Addressed remarks. The change record was added in #14.

Ready for a new round of review

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

Since #17 I've addressed a missed review remark.

nicxvan’s picture

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

I think this is ready, the last comment on the MR is addressed in #3580424: Stale references to defunct functions in field module.

I reviewed all of the commits since the ones I applied and I think all feedback has been addressed.

amateescu’s picture

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

Went through the MR and it looks great! Left a code suggestion on the form constructor change because I can't see why we wouldn't use property promotion there. Feel free to self-RTBC after handling that :)

nicxvan’s picture

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

Chatted in slack, that was intentional since it makes contrib BC harder, we just add property promotion for it when we drop the deprecation.

  • amateescu committed fcb3dff5 on main 
    task: #3572173 Deprecate field.module remaining functions

By: claudiu....
amateescu’s picture

amateescu commented
Status: Reviewed & tested by the community » Patch (to be ported)

Even though I started this MR originally, enough people have reviewed and worked on it since, so I'm comfortable merging it :)

Committed fcb3dff and pushed to main. Will need a reroll for 11.x..

nicxvan opened merge request !15343

amateescu’s picture

amateescu commented

\Drupal\Tests\node\Functional\NodeCreationTest::testAuthoredDate() fails consistently on CI as well as locally, but I think that's a problem with that specific test than the backport MR.

In any case, I'd prefer not breaking HEAD on my second day, so let's wait a bit until we can figure out the issue.

  • amateescu committed 22d7c626 on 11.x 
    task: #3572173 Deprecate field.module remaining functions

By: claudiu....
amateescu’s picture

amateescu commented
Status: Patch (to be ported) » Fixed

The test passes now both on CI and my local, must've been a time issue.

Committed 22d7c62 and pushed to 11.x. Thanks!

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.

amateescu closed merge request !15343

amateescu closed merge request !14712

Status: Fixed » Closed (fixed)

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