[19:40:21] So this is happening in 20 minutes :) [19:51:28] Wikimedia Code-Review Office Hours every Thursday at 20:00-21:00 UTC. (https://phabricator.wikimedia.org/E200) | Channel logs: http://ur1.ca/ov5sb | People with core+2 voiced [19:59:31] So, do we have any reviewers or reviewees today? [20:00:01] I'm willing to review backend changes [20:01:42] So Danny_B suggested some patches on https://phabricator.wikimedia.org/E200 [20:03:31] Hi would someone be able to review https://gerrit.wikimedia.org/r/#/c/291671/ please. [20:03:37] * bawolff amends his previous statement to, I'm willing to code review backend patchs that aren't super-bit rotted [20:04:02] Since im trying to get tests working on that repo that is currently deployed on wikimedia [20:04:24] I have another patch to rename the extension but that requires a bit more review due to big code changes. [20:04:35] twentyafterfour bawolff ^^ [20:05:09] missing tto [20:05:28] The -1 on there is about renaming the extension which im doing seperatly and has been waiting a while for review as we only want the tests to run i did that quickly. [20:07:12] thedj: T47850 [20:07:12] T47850: Code editor should not be cuffed to the enhanced toolbar - https://phabricator.wikimedia.org/T47850 [20:07:26] paladox: I left a comment on that patch [20:07:37] bawolff thanks [20:08:07] I also kind of agree with Dereckson, if we're going to start renaming stuff, it might be better to just fully make the extension be inline with modern conventions [20:08:30] bawolff I thought you carn't have Timeline and timeline [20:08:36] due to same name just capital at the front [20:08:51] hmm. Maybe windows wouldn't like that, I'm not sure [20:08:57] its fine on linux [20:09:00] bawolff Done at https://gerrit.wikimedia.org/r/#/c/243936/ [20:09:18] I guess that raises the question of if we need to keep windows compat [20:09:29] bwolff : yes. [20:10:40] bawolff not really any more microsoft brought ubuntu to windows. [20:11:04] yeah, but nobody runs their web server on their bash compatibility shim [20:11:16] Which means all the user has to do is start bash and then it will work like ubuntu [20:11:26] bawolff i have been running my test website on bash. [20:12:33] bawolff http://www.test-random-wikisaur.tk/ has been running on windows. [20:12:36] bash [20:12:58] That's not something we can reasonably tell people to do for production wikis [20:13:17] Then again, we don't reasonably support production quality wikis on windows anyways... [20:13:22] (03PS1) 10EBernhardson: Don't lose namespace when searching via api [core] - 10https://gerrit.wikimedia.org/r/292422 [20:13:38] Oh, microsoft really need to support symblinks in windows [20:13:39] For all I know, file system might support that on windows anyways, and just be case-insensitive for searching for files or something [20:13:44] I don't think it's reasonable to support windows in general ;) [20:13:50] but I'm just being a hater now [20:13:56] lets hope they do add the support. [20:14:00] (03CR) 10EBernhardson: "I think this will fix the broken browser tests in CirrusSearch that test the commons functionality." [core] - 10https://gerrit.wikimedia.org/r/292422 (owner: 10EBernhardson) [20:14:21] afaik windows file systems are truely case insensitive [20:14:26] twentyafterfour since microsoft added bash to windows ive been using it every day normaly i never used the command line but now i use bash. [20:14:29] :) [20:14:54] twentyafterfour Oh, my files are in lower case and Higher case. [20:15:04] paladox: what happens inside bash for windows, if you try to create Test.txt and test.txt in the same directory? [20:15:19] Ok let me go and do that. [20:15:40] if they are the same file then it's case insensitive, if they are two distinct files then it's case sensitive. [20:16:05] twentyafterfour it works [20:16:14] One's lower case and ones higher case [20:16:22] hmmm, interesting [20:17:02] And im also using file explorer. One thing i carn't currently do and it's annoying is use a windows application to edit files in [20:17:09] C:\Users\user\AppData\Local\lxss\rootfs\ [20:18:01] twentyafterfour microsoft havent modifyed the ubuntu image it is exactly the same image from ubuntu. [20:19:48] twentyafterfour But in apache2 you would need to be in bash or symblinks wont work [20:20:01] I can use cp in bash to copy them over to my main windows files. [20:20:08] located in /mnt/c/ [20:22:28] I see, so it's using a linux filesystem image [20:22:41] that's why files are case sensitive, it's not on the native windows volume [20:24:19] twentyafterfour actually it is on a native windows volume [20:24:39] Just seperate and yes i carn't seem to have both Test.txt and test.txt in my main windows files [20:24:58] But i can in C:\Users\user\AppData\Local\lxss\rootfs [20:25:04] I get sudo and apt-get [20:25:22] I started gvim and its gui is really old. [20:25:31] twentyafterfour ^^ [20:26:38] Its not a vm or anything its running of the lxss [20:26:39] https://msdn.microsoft.com/en-us/commandline/wsl/about [20:27:41] paladox: yes but the files are stored separately in a disk image not in windows filesystem [20:28:03] Oh, i can access them through file explorer. [20:28:18] And i can copy them to windows. [20:28:58] twentyafterfour i found this error https://github.com/Microsoft/BashOnWindows/issues/443 [20:30:04] (03CR) 10DCausse: [C: 032] Don't lose namespace when searching via api [core] - 10https://gerrit.wikimedia.org/r/292422 (owner: 10EBernhardson) [20:30:06] paladox: bash-on-windows is off-topic for code review beyond the question of case sensitivity... [20:30:06] twentyafterfour also someone managed to get another linux distro working but took along time to do it. Microsoft may open source the tech behind it too. [20:33:57] (03PS1) 10EBernhardson: Push common search api parameters into SearchApi class [core] - 10https://gerrit.wikimedia.org/r/292477 [20:34:42] (03PS2) 10EBernhardson: Push common search api parameters into SearchApi class [core] - 10https://gerrit.wikimedia.org/r/292477 [20:36:24] (03PS1) 10Gergő Tisza: AuthManager::setDefaultUserOptions and LoginForm::initUser shouldn't invalidate CA tokens [core] (REL1_27) - 10https://gerrit.wikimedia.org/r/292479 (https://phabricator.wikimedia.org/T136834) [20:36:41] (03CR) 10Gergő Tisza: [C: 032] AuthManager::setDefaultUserOptions and LoginForm::initUser shouldn't invalidate CA tokens [core] (REL1_27) - 10https://gerrit.wikimedia.org/r/292479 (https://phabricator.wikimedia.org/T136834) (owner: 10Gergő Tisza) [20:37:16] (03CR) 10Brian Wolff: [C: 04-1] "Looks good. One very minor issue with the messages." (032 comments) [core] - 10https://gerrit.wikimedia.org/r/8036 (https://phabricator.wikimedia.org/T12817) (owner: 10Chughakshay16) [20:38:30] Ok, one down. What's next? [20:38:42] bawolff: nice... [20:39:12] anyone here with patches to review? It seems that I need to do more work promoting this event outside of the core developers [20:40:07] twentyafterfour: mail reminder to nominees might be helpful [20:40:13] twentyafterfour Maybe a banner on mediawiki. But also create a page on mediawiki for users who doint want to use irc can suggests [20:40:17] patch to be reviewed [20:40:31] (03Merged) 10jenkins-bot: Don't lose namespace when searching via api [core] - 10https://gerrit.wikimedia.org/r/292422 (owner: 10EBernhardson) [20:40:57] paladox: patches are nominated on event's page in phab actually [20:41:10] paladox: part of the idea here is to have the author and reviewer in the same place at the same time [20:41:17] that place is irc [20:41:21] Oh ok [20:41:46] Maybe advertise on mediawiki with banners and on the main page [20:41:46] that's not always necessary but it's helpful [20:41:50] Ok [20:43:03] Low priority, but if someone could take a peek at https://gerrit.wikimedia.org/r/#/c/279956/ and give some feedback - it would be much appreciated [20:44:12] (03CR) 10Paladox: "recheck" [core] - 10https://gerrit.wikimedia.org/r/279956 (https://phabricator.wikimedia.org/T18922) (owner: 10Ferveo) [20:44:15] ferv: Ok, looking at it [20:44:18] (03CR) 10EBernhardson: Push common search api parameters into SearchApi class (031 comment) [core] - 10https://gerrit.wikimedia.org/r/292477 (owner: 10EBernhardson) [20:44:42] (03CR) 10jenkins-bot: [V: 04-1] Push common search api parameters into SearchApi class [core] - 10https://gerrit.wikimedia.org/r/292477 (owner: 10EBernhardson) [20:44:45] (03Merged) 10jenkins-bot: AuthManager::setDefaultUserOptions and LoginForm::initUser shouldn't invalidate CA tokens [core] (REL1_27) - 10https://gerrit.wikimedia.org/r/292479 (https://phabricator.wikimedia.org/T136834) (owner: 10Gergő Tisza) [20:45:31] (03CR) 10Bartosz Dziewoński: "Neat, I didn't expect you to go and implement the whole thing :D But, I don't think you can put the 'type' => 'reset' attribute on links ;" [core] - 10https://gerrit.wikimedia.org/r/292360 (https://phabricator.wikimedia.org/T136809) (owner: 10Gergő Tisza) [20:47:21] ferv: That's actually not going to work on wikis with $wgMiserMode enabled [20:47:59] Since the queries are done once and shown to multiple users, instead of doing it once per user [20:48:36] Insetad you have to use preprocessResults() to find if the item is on the user's watchlist [20:49:44] (03PS1) 10Gergő Tisza: Fix copy-paste mistake in HTMLForm::getButtons [core] - 10https://gerrit.wikimedia.org/r/292480 [20:49:53] (03CR) 10Gergő Tisza: "Oops. Fixed in I7bdc3a899d091c16ae8407351198fb11efaaefe8." [core] - 10https://gerrit.wikimedia.org/r/292360 (https://phabricator.wikimedia.org/T136809) (owner: 10Gergő Tisza) [20:51:30] (03PS1) 10Florianschmidtwelzow: HTMLForm: Don't add a type=reset to links [core] - 10https://gerrit.wikimedia.org/r/292481 [20:52:12] (03CR) 10Florianschmidtwelzow: [C: 04-1] "If88c80a46cba9729a9503b82532584443d6d5ba4 Oops, to slow :(" [core] - 10https://gerrit.wikimedia.org/r/292480 (owner: 10Gergő Tisza) [20:52:18] bawolff: Ahh... Well, I just learned something new. Thanks a lot. I'll do more research on this to see what would be the best approach. Thanks for the pointers [20:52:50] I'm going to repeat what i said on the gerrit change, just for the record [20:53:22] (03CR) 10Bartosz Dziewoński: [C: 032] HTMLForm: Don't add a type=reset to links [core] - 10https://gerrit.wikimedia.org/r/292481 (owner: 10Florianschmidtwelzow) [20:53:50] (03Abandoned) 10Bartosz Dziewoński: Fix copy-paste mistake in HTMLForm::getButtons [core] - 10https://gerrit.wikimedia.org/r/292480 (owner: 10Gergő Tisza) [20:54:03] (03CR) 10Bartosz Dziewoński: "Thanks for the quick response, both of you :D" [core] - 10https://gerrit.wikimedia.org/r/292480 (owner: 10Gergő Tisza) [21:00:25] (03CR) 10Brian Wolff: [C: 04-1] "Just to repeat what I said on irc. On wikis with $wgMiserMode = true; (including wikimedia wikis) these query pages are generated by the s" (031 comment) [core] - 10https://gerrit.wikimedia.org/r/279956 (https://phabricator.wikimedia.org/T18922) (owner: 10Ferveo) [21:01:30] (03Merged) 10jenkins-bot: HTMLForm: Don't add a type=reset to links [core] - 10https://gerrit.wikimedia.org/r/292481 (owner: 10Florianschmidtwelzow) [21:04:17] (03CR) 10Florianschmidtwelzow: "Thanks MatmaRex :D" [core] - 10https://gerrit.wikimedia.org/r/292481 (owner: 10Florianschmidtwelzow) [21:14:59] (03CR) 10Brian Wolff: [C: 032] Show ParserOutput warning instead of on the actual page output for ignored display titles [core] - 10https://gerrit.wikimedia.org/r/290073 (https://phabricator.wikimedia.org/T135949) (owner: 10Glaisher) [21:15:36] hour over? [21:16:47] Luke081515: yes but it's not limited to just one hour, if you've got something I'm still around [21:17:08] (03PS3) 10EBernhardson: Push common search api parameters into SearchApi class [core] - 10https://gerrit.wikimedia.org/r/292477 [21:22:12] (03CR) 10jenkins-bot: [V: 04-1] Push common search api parameters into SearchApi class [core] - 10https://gerrit.wikimedia.org/r/292477 (owner: 10EBernhardson) [21:23:08] (03Merged) 10jenkins-bot: Show ParserOutput warning instead of on the actual page output for ignored display titles [core] - 10https://gerrit.wikimedia.org/r/290073 (https://phabricator.wikimedia.org/T135949) (owner: 10Glaisher) [21:24:54] bawolff: my idea was, that old patches == nobody reviewed them -> perhaps forgotten -> need review. actual patches are on the radar... [21:25:13] (re your comment on the event page) [21:25:20] Danny_B: And that's definitely true, and a good idea [21:25:57] but sometimes MediaWiki has changed so much since those patches were made, its hard to review them [21:26:06] i was trying to pickup mixture. some ancient, some new, which are urgent... [21:26:08] other times they might be forgotten because everyone is afraid of them [21:26:38] it's more difficult to figure out what we can do about the ones everyone is afraid [21:26:46] was any of my suggestions falling into this category? [21:26:49] that's the harder problem that's been bothering me a lot [21:31:17] (03CR) 10Krinkle: [C: 032] resourceloader: Strip leading BOM when concatenating files [core] (REL1_27) - 10https://gerrit.wikimedia.org/r/292175 (owner: 10Krinkle) [21:38:07] (03Merged) 10jenkins-bot: resourceloader: Strip leading BOM when concatenating files [core] (REL1_27) - 10https://gerrit.wikimedia.org/r/292175 (owner: 10Krinkle) [21:41:25] Danny_B: I think everyone is afraid of the scribunto file source patch [21:42:06] Liangent's patches well good idea, is a little bit bitrotted at this point, and kind of large to review just within the CR hour [21:42:20] and involves a major schema change [21:50:10] hm, everytime I do this, the bot quits actually. why? [21:54:14] twentyafterfour would you be able to review https://gerrit.wikimedia.org/r/#/c/243936/ please. [21:54:30] Im not sure how we can keep Timeline and create timeline.php [21:54:48] twentyafterfour do you have access to create repos on gerrit. [21:54:58] please [21:55:20] paladox: I don't know if I have access to create repos on gerrit [21:55:47] As an alternative view, why do we care about unit tests for Timeline. Its mostly written in perl after all [21:56:12] I have access to that at phabricator, but not at gerrit [21:56:17] So I can't help there [21:57:00] Oh ok [21:57:05] bawolff nope mostly php [21:57:13] since there is only one perl file i found [21:57:32] The php file is like 30 lines long. The perl file is like 3000 lines long [21:57:40] Oh [21:57:55] twentyafterfour do you see create project here https://gerrit.wikimedia.org/r/#/admin/projects/ [21:57:56] please [21:58:00] why does the uppercase filename break jenkins? [21:58:26] Im not sure i think jenkins expects the file to be same name as the project [21:58:47] But converting to extensions .json may fix things. [22:00:06] twentyafterfour this is where the file is located [22:00:06] https://phabricator.wikimedia.org/diffusion/CIJE/browse/master/mediawiki/conf.d/50_mw_ext_loader.php [22:00:24] Could you have a look to see if you can support both lower case and uppercase [22:00:27] please [22:01:32] twentyafterfour maybe this http://php.net/manual/en/function.ucfirst.php [22:02:01] bawolff: it would be actually good, if someone can update tasks of need-rebase patches with such note there. and also comment in gerrit i guess [22:02:41] Any task that was last modified more than 3 months ago definitely needs a rebase [22:03:07] if it's not an mostly inactive repo or a never changed file ;) [22:03:14] yeah [22:03:21] but for core, 3 months is a good rule [22:03:34] if it is we have the problem, that we have nearly no experts ;) [22:04:20] there is also that new tag #patch-needs-improvement [22:05:32] bawolff We could rename the repo to Timeline instead of timeline. [22:05:41] Renaming git repos is hard [22:05:44] That way no majour changes. [22:05:56] or so I'm told [22:06:01] I might be wrong on that [22:06:11] bawolff not really we can copy it into phabricator and then mirror to gerrit. [22:07:21] twentyafterfour looks like your an admin at https://gerrit.wikimedia.org/r/#/admin/groups/1,members [22:07:30] which means i think you can create repos in gerrit [22:48:45] bawolff Hi could you review https://gerrit.wikimedia.org/r/#/c/247085/ please. [22:48:50] sql related [22:51:00] and could this be https://gerrit.wikimedia.org/r/#/c/290070/ review [22:51:02] please [22:51:06] twentyafterfour ^^