[18:43:41] zabe: I recommend you talk to urbanecm about your CentralAuth patch. It looks good to me, but I haven't tested it, and I don't really want to set up CA locally... [18:44:38] duesen: CA isn't /that/ hard to set up! [18:46:25] taavi: last time it took me a couple of hours with a lot of help from urbanecm. it's doable, but not something i want to spend time on right now, if others have a setup ready that they could use for testing [18:49:39] which patch are we talking about? [18:51:12] urbanecm: oh hey! i think zabe may have missed my message, he just left... Anyway, this one: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CentralAuth/+/756137 [18:51:42] https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CentralAuth/+/756137 (which has an open question from me) [19:00:48] taavi: The problem is using two different values to represent the same wiki. Using false to mean "this wiki" has a decade of history and is hardcoded all over the code base. Using the wiki's real ID instead will lead to errors when a wiki-aware value object tries to ensure consistency and refuses to return an ID that belongs to a mismatching wiki. [19:01:19] So we need to normalize, one way or the other. But value objects can't do that, they cannot access services. We can only do this in services before creating and accessing value objects. [19:03:12] CentralAuthUser of course isn't a value object, but it isn't a service either. It's an old school monster object, which needs to access services, but doesn't have a good way to inject them. [20:41:37] duesen: hey, I mainly wanted to know whether this is the approach we want to take here. [20:41:54] I have tested the change locally and it does seem to work for me [21:13:04] zabe: yea, but it's always good to have someone who will actually +2 your patch ;) [21:14:24] yep, that's useful ;) [21:22:37] * taavi does [21:25:23] thx [21:27:16] did you have any other patches you wanted me to review? I've fallen totally behind on code review backlogs [21:33:20] no :)