getClientIPfromXFF has a comment in it's PHPDoc for it to be moved out of the Hooks.php file. It's not a hook, and is not exclusively used by code in Hooks.php. As such, I agree with this comment and it should be moved into a utility class. Taking inspiration from CentralAuth, a utility service for CheckUser would be a useful way to achieve this. Other extensions also use getClientIPfromXFF and moving to a service would still allow them to access the code.
Description
Description
Details
Details
Related Objects
Related Objects
Event Timeline
Comment Actions
Change 873053 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):
[mediawiki/extensions/CheckUser@master] Move getClientIPfromXFF to a new CheckUser utility service
Comment Actions
Change 873059 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):
[mediawiki/extensions/AccountInfo@master] Replace deprecated CheckUserHooks::getClientIPfromXFF with service
Comment Actions
Change 873053 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Move getClientIPfromXFF to a new CheckUser utility service
Comment Actions
Taking inspiration from CentralAuth, a utility service for CheckUser would be a useful way to achieve this
In retrospect, a single "utility service" to collect various separate miscellaneus utilities was a bad idea (see T288084). I would suggest instead having a separate service class for each separate group of functionality.
Comment Actions
Change 873059 merged by jenkins-bot:
[mediawiki/extensions/AccountInfo@master] Replace deprecated CheckUserHooks::getClientIPfromXFF with service
Comment Actions
While I agree with this in principle, I think there just aren't that many methods in CheckUser that could end up in a utitlity service. However, we could have perhaps given the service introduced here a better name.
Comment Actions
It's been moved and all repos on gerrit have had the deprecated method updated. The other non-archived github repo works on release versions of mediawiki, so this can be left to later.
Removing the now deprecated method can be done later, but I'm thinking waiting for the next release to go out would be best. Either way that can be done in a different task.