Remove UserIdentity::getUserId. Use UserIdentity::getId instead.
Description
Description
Details
Details
Customize query in gerrit
Related Objects
Related Objects
Event Timeline
Comment Actions
Change 666411 had a related patch set uploaded (by Vadim Kovalenko; owner: Vadim Kovalenko):
[mediawiki/core@master] Avoid using UserIdentity::getUserId
Comment Actions
Change 666411 merged by jenkins-bot:
[mediawiki/core@master] Avoid using UserIdentity::getUserId
Comment Actions
Change 667991 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Replace getUserId() with getId()
Comment Actions
Change 668033 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/core@master] Re-introduce UserIdentity::getUserId - removed in a violation of SIP
Comment Actions
Change 667991 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Replace getUserId() with getId()
Comment Actions
Thanks for pointing this out @Tgr - I'm here because SecurePoll was one of those extensions.
I appreciate that getUserId wasn't covered by the stable interface policy yet, but wondering what we should do to avoid this in the future... Should extension authors to keep using the older, deprecated method until the new one has been included in a stable release, or should whoever removes the new method check for extension uses first?
Comment Actions
Change 668033 merged by jenkins-bot:
[mediawiki/core@master] Re-introduce UserIdentity::getUserId - removed in a violation of SIP
Comment Actions
The fix has been partially reverted due to breaking extensions. https://gerrit.wikimedia.org/r/c/mediawiki/core/+/668033
We need to find all the usages in the extensions before removing methods. Let's try again.
Comment Actions
Removing deprecated methods happens all the time. Unding an interface change is pretty rare. So the latter makes more sense IMO.
FWIW I did check via codesearch and gerrit earlier and didn't found any other usage than GrowthExperiments and SecurePoll (both fixed now) so I don't think a revert is necessary.
Comment Actions
Change 669824 had a related patch set uploaded (by Vadim Kovalenko; owner: Vadim Kovalenko):
[mediawiki/core@master] Avoid using UserIdentity::getUserId
Comment Actions
Change 669836 had a related patch set uploaded (by Vadim Kovalenko; owner: Vadim Kovalenko):
[mediawiki/extensions/AbuseFilter@master] Replace UserIdentity::getUserId with ::getId
Comment Actions
Change 669836 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Replace UserIdentity::getUserId with ::getId
Comment Actions
Change 669824 merged by jenkins-bot:
[mediawiki/core@master] Avoid using UserIdentity::getUserId
Comment Actions
What was the reasoning for this back and forth? I can't find much of an explanation anywhere.
getUserId() was introduced and getId() deprecated in https://gerrit.wikimedia.org/r/658751 in 2021-02-02, linked to T260933: Make UserIdentity objects aware of which wiki they belong to. This ticket sounds like it is unrelated. Why is that?
The deprecation of getId() was finally undone in https://gerrit.wikimedia.org/r/669824 in 2021-03-10, linked to this ticket here. Again with no explanation.
Personally I agree that getId() is the better choice for something that's already called UserIdentity. Not only that, it's used thousands of times in existing code. Changing e.g. $user->getId() to $user->getUserId() doesn't really make anything better.
Comment Actions
What was the reasoning for this back and forth? I can't find much of an explanation anywhere.
You are right, trying to change it was a mistake. I think now the mistake was corrected and we fully rolled it all back, so I'll resolve this ticket.
The logic behind the original switch was that UserIdentity has been initially named incorrectly. In reality it's not UserIdentity, but ActorIdentity. We were thinking to rename it, in which case the getId() in its current form would become very odd, but then decided against that and rolled back all the changes.