Page MenuHomePhabricator

Deprecate Entity methods not in EntityDocument
Closed, ResolvedPublic1 Estimated Story Points

Description

Had a chat with @Bene. This is what's left in Entity and not deprecated:

  • Entity::getFingerprint already has an interface, FingerprintProvider. In 3.1, the method should be deprecated in Entity and un-deprecated in Item and Property (via duplicate implementations, similar to Item::setLabel and such).
  • Entity::setFingerprint needs investigation. The most flexible solution is to create a FingerprintHolder with a getter and a setter, similar to StatementListHolder, make Item and Property implement this interface and deprecate the methods in Entity. But do we really need setFingerprint? If getFingerprint is guaranteed to not return a copy, all manipulations can be done on the returned Fingerprint object and setFingerprint never needs to be called. What we need to do:
    • Check if all usages of setFingerprint (especially the ones in Deserializers) are optional and can be removed.
    • The guarantee that getFingerprint does not return a copy must be added to the methods documentation.
  • Entity::isEmpty should be deprecated and un-deprecated in Item and Property.
  • Entity::clear should be deprecated and later die. Just create a new Item if you need a new item. Repurposing existing entities is ugly.
  • Entity::copy. Not sure about this. It used a lot, more than 100 times, mostly in tests obviously but also in production. But it's very, very trivial and should be avoided anyway. Better create a new object.

Related Objects

StatusSubtypeAssignedTask
Declineddchen
OpenNone
OpenNone
DuplicateNone
OpenNone
OpenNone
DuplicateNone
ResolvedNone
ResolvedNone
ResolvedNone
DuplicateNone
InvalidLydia_Pintscher
OpenNone
OpenNone
StalledNone
OpenNone
ResolvedAddshore
Resolvedthiemowmde
ResolvedAddshore
ResolvedBene
DeclinedBene
ResolvedBene

Event Timeline

Bene claimed this task.
Bene raised the priority of this task from to Medium.
Bene updated the task description. (Show Details)
Bene added subscribers: JanZerebecki, daniel, JeroenDeDauw and 3 others.

Change 222934 had a related patch set uploaded (by Bene):
Move isEmpty and isStub to Item/PropertyContent

https://gerrit.wikimedia.org/r/222934