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

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?)

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)

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