Support from Acquia helps fund testing for Drupal Acquia logo

Comments

effulgentsia’s picture

Issue tags: +Entity Field API

Tagging.

Status: Needs review » Needs work

The last submitted patch, entity-bc-removal-from-test-entities.patch, failed testing.

Berdir’s picture

Wondering why we have EntityCacheTest and EntityTestCache...

Ah, EntityCacheTest is used in that weird EntityApiInfoTest that tests entity info altering and module enable crazyness. Should be possible to remove that and switch another of our test entities...

And FieldUITestNoBundle was added in #1986888: Field UI integration for entity types is brittle.

Note that just extending from EntityNG and using the NG storage is somewhat weird, because that means it is impossible to actually use those entity types (was already not really possible before).

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
1.41 KB
2.4 KB

Let's see if this at least lets the tests run.

Status: Needs review » Needs work

The last submitted patch, entity-bc-removal-from-test-entities-4.patch, failed testing.

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
710 bytes
3.26 KB

Re #3, I agree these test classes seem a bit superfluous, but not sure how to best remove them. If that's easy to do, please post a patch for it. Otherwise, just trying to remove the blockers to #1983554: Remove BC-mode from EntityNG .

Berdir’s picture

Not sure if it's technically a blocker for removing the BC decorator, the BC decorator is only used for already converted entity types that still need to be accessed the old way.

But it certainly makes sense, especially given the direction I'm moving to in #2004244: Move entity revision, content translation, validation and field methods to ContentEntityInterface (Which basically makes EntityNG ContentEntityBase and will limit a number of features to ContentEntityInterface, like being able to be fieldable, translatable, ...). Will try to post an updated issue summary there ASAP.

Will RTBC this once it's green, we can still think about re-using other entity types in non-critical issues.

effulgentsia’s picture

Will RTBC this once it's green

#6 is green :)

Berdir’s picture

Yeah, I know. The problem is that I would have to change everything you're changing here again in #2004244: Move entity revision, content translation, validation and field methods to ContentEntityInterface (at least the entity classes), so this is somewhat pointless. Might be easier to just merge into that from the start. Or find a way to drop those types.

effulgentsia’s picture

Status: Needs review » Postponed

Ah, ok. This isn't urgent, can postpone on that issue.

Berdir’s picture

Priority: Critical » Normal
Status: Postponed » Needs work

So, the BC mode is gone and the issue that I referenced is in, so this is no longer blocked and also no longer critical.

It also means we can aim for a nice solution instead :)

Berdir’s picture

Issue summary: View changes
Status: Needs work » Closed (duplicate)

Those two no longer exist and/or have been converted elsewhere/otherwise.