[10:52:01] Have we got anywhere with the job issue from last weeks train deployment? [14:52:07] legoktm: About? [14:52:13] hey [14:52:19] I'm here for another 10 minutes [14:52:57] So.. moving fr to extreg caused a load of fallout (unsurprisingly as the config is complex) [14:53:18] I thought the easiest way to fix this short term was to stick it in an extensionfunction in flaggedrevs.php [14:53:27] and defer the overrides, merging and stuff until the extension is loaded [14:53:55] however, this new callback ends up later in wgExtensionFunctions than the FR one (on enwiki, FR ends up first in the list?) [14:54:16] so it runs too late to change any config before FlaggedRevsHooks::onExtensionFunctions runs... And does conditional config based on it [14:54:21] Any bright deas what to do? [14:56:28] so we want the order to be 1) extension.json 2) flaggedrevs.php EF, 3) FlaggedRevsHooks::onExtensionFunctions ? and right now its running in 1 3 2? [14:56:39] basically, yes [14:57:29] It's intriguing how all the extensions end up higher in the array.... [14:57:47] https://phabricator.wikimedia.org/P8646 [15:00:02] To add to the hacks I could add a hook to FlaggedRevsHooks::onExtensionFunction to use instead of extensionfunctions [15:00:03] * Reedy barfs [15:02:37] I guess it's due to array_merge_recursive [15:05:34] I can only come up with really shitty hack ideas [15:05:54] heh [15:06:11] I think we might be better off temporarily reverting the FR config stuff that isn't working properly back into the PHP file [15:06:21] it's all a mix of a few limitations in extreg, and the excessive complexity of FR config [15:08:20] for the issue here... enwiki is reporting [15:08:26] 6 other wikis use $wgFlaggedRevsProtection = true [15:10:22] I guess have a look for these other weird config vars used for conditional config/hooks... [15:10:29] And pull them out of the callback [15:10:56] Which just splinters the config more than it currently is [15:12:28] Calling the extension function twice... Isn't going to be that expensive [15:12:53] but will result in dupes in array[] = cases [15:15:06] Anything that's a simple bool should be ok out of the closure though [15:15:09] Let's do that for starters [15:15:50] * Reedy mumbles something about just undeploying FR [15:22:23] I'm so tempted to pull out half of this config into IS [15:32:11] woah, you are trying to convert FR to extension registration? [15:32:21] hardcore. [15:33:09] bit of a hack but SetupAfterCache runs (way) before ExtensionFunctions [15:33:54] There's also onRegistration from extension registry. [15:33:58] could fit in somehow [15:34:03] might* [15:38:15] tgr: Trying? It's been merged a while :) [15:38:41] onRegistration isn't a hook [15:38:46] well, not in the traditional sense [15:40:16] short break to empty my washing machine and hang some clothes up [15:40:22] Then I'll do some more cleanup [17:47:29] joy [17:47:35] FR uses $wg that it doesn't define [18:09:34] https://gerrit.wikimedia.org/r/518781 [18:09:36] what a mess [18:12:15] Stuff that's been deprecated for 9 years [18:21:42] I guess we need to ask the hewikisource community what they want doing with that? [18:22:24] Reedy: Is it not a straight-forward config format migration? [18:22:32] Afaik it doesn't control different behaviour, right? [18:22:44] hewikisource [18:22:45] $wgFlaggedRevsTags = [ 'completeness' => 3, 'accuracy' => 3, 'formatting' => 3 ]; [18:22:57] everwhere else... [18:22:58] $wgFlaggedRevsTags = [ [18:22:58] 'status' => [ 'levels' => 1, 'quality' => 2, 'pristine' => 3 ], [18:22:58] ]; [18:23:09] or some variant thereof [18:23:13] Unless I'm missing something [18:23:20] I've been starting at FR stuff most of the afternooon [18:23:28] right, so 'completeness' => 3 becomes an array like 'completeness => [ 'levels' => 3, … ] I guess [18:23:36] or whatever the bc code was expanding it to [18:23:57] the int becomes the 'quality' field it looks like [18:24:03] $ratingLevels = $wgFlaggedRevValues ?? 1; [18:24:03] $minQL = $levels; // an integer [18:24:03] $minPL = $wgFlaggedRevPristine ?? $ratingLevels + 1; [18:24:22] wgFlaggedRevValues is 4 [18:24:27] wgFlaggedRevPristine isn't set [18:24:40] .. usually [18:24:59] but yeah, all three expand to something today, so that can keep. [18:25:03] sure [18:25:09] I was offering it as potential values [18:25:12] it's likely not what they community wants it to be, so good to ask because it probably broke some years ago [18:25:21] but we don't have to change it today, it can keep working without the bc code [18:25:29] There's also use of different... tag keys in different wikis [18:25:48] Krinkle: Actually [18:25:50] You missed some [18:25:52] $wgFlaggedRevsTags['accuracy']['levels'] = 1; [18:26:09] Those are similarly broken [18:26:13] not per se. [18:26:16] * Reedy gets staby [18:26:20] The value of tags['accuracy'] is $levels [18:26:26] which is an array in this example, so fine [18:26:30] the var name is bad [18:26:36] the whole extension is bad [18:27:19] well, if you're volunteering to take ownership, I hear it's looking for a steward. [18:27:25] :D [18:28:06] If by take ownership, you mean force undeployment [18:28:07] Sure [18:28:32] well, undeployment will take some time and migration, but yes, sunsetting is a power that a steward can take. Not even joking. [18:29:03] migration to what? [18:29:08] there's literally no alternative [18:29:33] I could write a bunch of random code that has no alternative either, just because it can exist and once did, doesn't mean it has to. [18:30:02] Removal of it is technically easy, politically hard [18:30:02] I'm also not saying it doesn't. Just saying there are options. Up to product in the end I suppose. [18:30:12] I'm sure product DGAF [18:30:17] They're not gonna put any resources into it [18:31:33] Yep, and that's the wonderful joy of letting go and allowing ourselves to learn the hard way if the easy way doesn't work. [18:33:20] Reedy: If you can, do track the hours you and James spend on this and document at https://phabricator.wikimedia.org/T185664, including why (ext migration) [18:33:32] "lots" [18:33:39] wfm :) [18:34:10] I must be on 5 or 6 hours today alone [18:52:07] Krinkle: https://phabricator.wikimedia.org/T226439#5279867 yay reflection [19:02:49] https://gerrit.wikimedia.org/r/518789 should be good to go though [19:28:29] Krinkle: helps if I load the config... [19:38:09] thanks :) [19:39:33] I'm loving this patches getting merged within 10 minutes of CR+2 [19:40:16] not bad [19:40:36] https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/FlaggedRevs/+/518789/ clocked at 6 minutes [19:40:43] not gated though [19:40:49] I mean shared wmf gate [19:43:41] sub 10 minutes is much better than 40-50 :P [20:00:00] some of that FR deprecated code.. [20:00:02] is actually 11+ years [20:00:05] https://github.com/wikimedia/mediawiki-extensions-FlaggedRevs/blame/f2d4e4d4a19bd31295d6d97b7ff2d6a9983d6071/backend/FlaggedRevs.class.php#L104 [22:03:34] Reedy: FR is such high quality, well-maintained code! [22:03:40] ikr?