Page MenuHomePhabricator

Remove unused arguments from TermValidatorFactory::getFingerprintValidator
Closed, ResolvedPublic

Description

  • TermValidatorFactory::getFingerprintValidator() has two arguments ($entityType and entityId $entityId) that are now unused
  • TermValidatorFactory::getFingerprintUniquenessValidator(), a separate method, also exists; its documentation still talks about the old and new term store, but the old term store is no more
  • the FingerprintValidator interface (returned by getFingerprintValidator(), but not by getFingerprintUniquenessValidator()) is documented as “intended particularly for uniqueness checks”, but none of the remaining implementations check uniqueness (that was “old term store stuff”)
  • the CompositeFingerprintValidator class, one of the implementations of that interface, is unused and can be removed
  • the NullFingerprintValidator class, another of the implementations of that interface, is unused except in tests
  • (there is only one other class implementing that interface, LabelDescriptionNotEqualValidator, so maybe the interface can be removed completely?)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 12 2020, 4:08 PM

Hm, I think there are a few more things to clean up in this area:

  • TermValidatorFactory::getFingerprintValidator() has unused parameters (original task description)
  • TermValidatorFactory::getFingerprintUniquenessValidator(), a separate method, also exists; its documentation still talks about the old and new term store, but the old term store is no more
  • the FingerprintValidator interface (returned by getFingerprintValidator(), but not by getFingerprintUniquenessValidator()) is documented as “intended particularly for uniqueness checks”, but none of the remaining implementations check uniqueness (that was “old term store stuff”)
  • the CompositeFingerprintValidator class, one of the implementations of that interface, is unused and can be removed
  • the NullFingerprintValidator class, another of the implementations of that interface, is unused except in tests
  • (there is only one other class implementing that interface, LabelDescriptionNotEqualValidator, so maybe the interface can be removed completely?)
toan claimed this task.Oct 13 2020, 7:23 AM
toan moved this task from To do to Doing on the Wikibase wb_terms leftovers 2020 board.

Change 633745 had a related patch set uploaded (by Tobias Andersson; owner: Tobias Andersson):
[mediawiki/extensions/Wikibase@master] wip: Cleanup TermValidatorFactory

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

toan updated the task description. (Show Details)Oct 13 2020, 2:22 PM
toan updated the task description. (Show Details)

Change 633942 had a related patch set uploaded (by Tobias Andersson; owner: Tobias Andersson):
[mediawiki/extensions/Wikibase@master] Remove unused validators

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

Change 633961 had a related patch set uploaded (by Tobias Andersson; owner: Tobias Andersson):
[mediawiki/extensions/Wikibase@master] Update labelDescription validator usage

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

Change 634046 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Remove outdated parts of change op comments

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

Change 633745 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Cleanup TermValidatorFactory

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

Change 633942 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Remove unused validators

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

Change 633961 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Update LabelDescriptionNotEqualValidator usage

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

Change 634046 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Remove outdated parts of change op comments

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

Change 634272 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Remove a TODO that no longer applies

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

Looked through the validator factory and found two more things, with patches. Remove unused parameter is not really related to this task (the parameter had been unused for much longer), but remove a TODO is, so moving back to review for (hopefully) just a short while.

Change 634272 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Remove a TODO that no longer applies

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

WMDE-leszek closed this task as Resolved.Oct 16 2020, 9:12 AM
WMDE-leszek moved this task from Verification to Done on the Wikibase wb_terms leftovers 2020 board.