[00:08:56] Hydronium: no idea, really. i haven't looked into this any further yet [00:09:54] eh, what could go wrong [00:10:02] *sun explodes* [00:16:39] Actually, Newpages, Newfiles, and Recentchanges are remarkably similar [00:17:02] Newpages is just Recentchanges filtered to new pages, and newfiles is newpages filtered to File: namespace [00:17:15] Recentchanges extends ChangesListSpecialPage [00:25:14] So to make use of the oojs ui date selector widget maybe there should be a custom "Since date ___" option next to 1, 7, etc. [00:25:34] or just implement T120733 [00:27:55] I'll have Newpages base off of Recentchanges first [00:34:18] that sounds scary [00:40:33] Is PHPstorm any good? [00:43:05] eh, free student license, why not [00:50:23] 6Multimedia, 6Commons, 10MediaWiki-File-management, 10MobileFrontend, and 2 others: Explore lazy-loading WebP thumbnails to supporting browsers (eg Android Chrome) - https://phabricator.wikimedia.org/T114791#1896706 (10Jdlrobson) [00:58:33] marktraceur: Idioms are hard. My mum quizes me for Hindi idioms every time I go home just so that she can get her lulz. [00:58:43] MatmaRex: Good to merge? https://gerrit.wikimedia.org/r/#/c/260402 [01:00:23] morning prtksxna :o [01:00:31] prtksxna: should be, jenkins was broken earlier today [01:00:31] MatmaRex: o/ [01:02:25] MatmaRex: I'll +2 it, hopefully Jenkins will cooperate. [03:36:02] MatmaRex was right... 19:34 <+MatmaRex> that sounds scary [03:42:38] * Hydronium rewrites plans [04:10:31] HI, I'm a GCI student. Any tips for learning how MediaWiki's extensions work? Currently I'm browsing through the source code with the relevant page open and mentally debuggin' it.. [04:11:10] It's kinda slow. I don't know jQuery, I'm using my intuition and google to help [04:13:29] Here's what I'm trying to follow: [04:13:38] https://codein.withgoogle.com/tasks/5766197941370880/ [04:16:36] tgr: I think I've got it. Lemme push a patch. [04:23:08] * Hydronium stretches [04:32:27] I'm getting this error when I try to push a patch. https://dpaste.de/5Wof [04:36:09] Its best to add your ssh keys in the gerrit web interface, then you aren’t prompted for a username and password at all. [04:38:27] (03PS17) 10MtDu: Add gender support to Multimediaviewer-text-embed-credit-text-tbls and other messages [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) [04:38:49] I see you figured it out…I was going to suggest you check your username, but oh well :P [04:41:01] unicornisaurous: I just used a different machine. [04:48:14] 6Multimedia, 10MediaWiki-Uploading, 5MW-1.27-release-notes, 5Patch-For-Review, 5WMF-deploy-2016-01-12_(1.27.0-wmf.10): Cross-wiki upload tool must avoid to add empty language templates - https://phabricator.wikimedia.org/T121746#1897018 (10Prtksxna) 5Open>3Resolved [04:49:09] 6Multimedia, 6Commons, 10MediaWiki-Uploading, 7JavaScript, and 3 others: Cross-wiki-upload adds "null" as author - https://phabricator.wikimedia.org/T121097#1897020 (10Prtksxna) 5Open>3Resolved [06:23:45] Hydronium: PHPStorm is the least bad if you want a powerful IDE [06:23:59] powerful as in able to reason about semantics [06:24:11] it's friggin' slow [06:24:20] but the alternatives are even slower [06:25:32] JadeMaveric: that's a very generic question [06:25:45] what are you trying to learn? [06:37:07] tgr: Right now, how the different files in the Mediaviewer extension link to each other. [07:01:43] JadeMaveric: the files in resources/mmv/model are used to pass data around, mmv/provider classes are used to fetch data from external sources, mmv/ui contains the HTML generation logic, mmv.js puts them all together, mmv.bootstrap.js is loaded when the page is ready and loads mmv.js when MediaViewer is opened [07:02:39] most other files in /mmv are one-off things that did not really fit anywhere and legacy code that we never got around to refactor [07:03:58] the PHP file contains the little server side HTML generation logic that's needed, it's not too important [07:36:40] tgr: Thanks that cleared up a lot. I'll ask if I've run into any difficulties. btw: Should I entire remove the chunks of code or should I comment them out? [07:36:54] That's for T59308 [07:59:17] JadeMaveric: definitely remove [07:59:50] that's what version control software is for, to make old code findable [08:04:41] tgr: Right [08:13:27] Does the param: author refer to the artist or the uploader. I'm assuming the former, it seems like it. But then, what does authorCount stand for? [08:13:46] Does it come in if we have multiple authors? [08:36:55] JadeMaveric|away: yes [08:37:15] it's used for stuff like "by E. Elmer and 2 others" [08:37:47] e.g. when the image is a photograph of a painting, and both the photographer and the painter are authors in a sense [08:38:01] the uploader is called lastUploader [09:39:16] * JadeMaveric yawns! [09:40:24] tgr: Okay, I'm working on deleting sections of code that are related to the uploader. Anything I've got to bear in mind? So I don't break anything? [09:52:48] JadeMaveric: nothing I can think of [10:33:56] Something very strange is going on. The changes I make in src/mediawiki/mw.Upload.BookletLayout.js get reflected when I refresh, but not the changes in src/mediawiki/api/upload.js! [10:36:51] And a few other files :O [10:36:59] I might be losing my mind. [10:37:01] * prtksxna reboots [10:53:34] Its still happening. I get a better version when I remove debug=true, but its still happening. [10:53:44] At least now I know I am not editing a wrong file [11:04:20] It was chrome's fault :( [11:14:15] Now we're making progress :D [11:17:26] How do I test my changes? I've just enabled the mmv role on my vm. It's provisioning now. [11:37:19] JadeMaveric: Once the VM is done provisioning you should be able to refresh the relevant page and test your changes. [11:49:52] 6Multimedia, 6Commons: A new panoramic viewer for commons - https://phabricator.wikimedia.org/T105789#1897421 (10Aklapper) >>! In T105789#1765338, @dschwen wrote: > there is some work left, namely supporting arbitrarily high resolution panos. > Quite a few people know about this, but as it is not quite complet... [12:13:32] hi [13:47:09] MatmaRex: o/ [13:52:18] 6Multimedia, 10MediaWiki-Special-pages: Special:MediaStatistics: Show file size per section after the section title (before the table) - https://phabricator.wikimedia.org/T122208#1898261 (10Florian) [14:02:26] MatmaRex: I've added a progress bar for the mw.Upload.BookletLayout and showing ETA in mw.Upload.Dialog [14:02:46] Updating the mw.ForeignStructuredUpload.BookletLayout should be easy enough if you think the code is fine [14:03:07] 6Multimedia, 10MediaWiki-General-or-Unknown, 10VisualEditor, 10VisualEditor-MediaWiki-Media, 5Patch-For-Review: Show determinate progress bar for the image upload in mw.ForeignStructuredUpload.BookletLayout - https://phabricator.wikimedia.org/T115861#1898280 (10Prtksxna) a:3Prtksxna [14:08:28] Add the foreign booklet [14:08:35] I haven't tested it with VE yet :\ [14:20:55] *yawn* good morning all [14:22:11] morn [14:24:01] ooh, neat. looking. [14:32:03] marktraceur: by the way, bad news, we broke stuff by removing the input position polling thingie. https://phabricator.wikimedia.org/F3135345 [14:32:18] marktraceur: it's not updated to match the smaller button after you choose a file. [14:32:54] Huh, I tried to test that, maybe the later changes didn't go well [14:46:58] brion: which session do you think we should do first video/DOM/parsoid, or video/general ? [14:52:34] 6Multimedia, 6Commons, 10MediaWiki-File-management, 10Wikimedia-Video, 5Patch-For-Review: Experiment with a player for animated gif/png - https://phabricator.wikimedia.org/T101644#1898339 (10TheDJ) [15:02:08] I didn't realize the strategy meeting was going to be on Bluejeans :\ [15:04:47] 6Multimedia, 6Commons, 10MediaWiki-File-management, 10Wikimedia-Video, 5Patch-For-Review: Experiment with a player for animated gif/png - https://phabricator.wikimedia.org/T101644#1898372 (10TheDJ) [15:07:33] prtksxna: i like the progress bar :D [15:07:42] \o/ [15:12:45] MatmaRex: refactoring Newpages to extend Recentchanges was a terrible terrible idea [15:13:01] I'll just add day functionality to ReverseChronPager to start out [15:13:25] Should there be a separate class that takes in two dates? [15:40:05] 6Multimedia, 10MediaWiki-Special-pages: Special:MediaStatistics: Show file size per section after the section title (before the table) - https://phabricator.wikimedia.org/T122208#1898394 (10MarkTraceur) p:5Triage>3Low [16:01:09] MatmaRex? [16:01:37] oh. [16:01:41] :) [16:06:06] 6Multimedia, 6Commons, 10UploadWizard: Server side flickr review - https://phabricator.wikimedia.org/T89131#1898434 (10MarkTraceur) p:5Normal>3Low [16:08:45] MatmaRex: That i18n stuff was cool. I never knew about it. [16:12:04] :) [16:12:47] MatmaRex: Did you get a chance to test it in VE? [16:13:47] prtksxna: yeah, the progress bar wored just fine :D [16:14:21] Cool. Should be easy enough to update the dialog title if we wanted. [16:14:35] prtksxna: one funny thing i noticed is that if you make the dialog content high enough to need scrolling (by adding a long description and lots of categories), the progress bar will scroll out of view [16:14:55] which i guess makes sense, since it's part of the dialog content and not the dialog head. and shouldn't be an issue in practice [16:15:42] Hm. We shouldn't let that happen [16:16:39] i think avoiding this would require making it part of the dialog, and not the booklet, and that seems like it'll be a bit of a pain [16:17:29] Yeah [16:18:21] MatmaRex: Or we could set its position to fixed. That seems to work, sort of… [16:18:57] hmm, didn't seem to work for me when i tried. maybe i did something wrong [16:19:42] ugh, I think I set it on the bar, I'll try properly this time [16:26:46] MatmaRex: Yep, setting the height and position on oo-ui-progressBarWidget-bar works. z-index is messed up though. [16:28:35] I'll test with ve and append [16:29:20] s/append/amend [17:09:43] Restbase is failing agan :\ [17:58:48] 6Multimedia, 10MediaWiki-General-or-Unknown, 10VisualEditor, 10VisualEditor-MediaWiki-Media, 5Patch-For-Review: Show determinate progress bar for the image upload in mw.ForeignStructuredUpload.BookletLayout - https://phabricator.wikimedia.org/T115861#1898691 (10Esanders) The upload progress is one of the... [18:06:03] 6Multimedia, 10MediaWiki-General-or-Unknown, 10VisualEditor, 10VisualEditor-MediaWiki-Media, 5Patch-For-Review: Show determinate progress bar for the image upload in mw.ForeignStructuredUpload.BookletLayout - https://phabricator.wikimedia.org/T115861#1898710 (10Esanders) I think it's important that we ha... [18:09:04] 6Multimedia, 6Commons, 10UploadWizard: CC IGO 3.0 licences (BY & BY-SA) in the Wikimedia Upload wizard - https://phabricator.wikimedia.org/T121109#1898718 (10VictorGrigas) If this is the case, then under the user preferences for commons upload wizard (in user accounts) can there be a 'more' tab for the 516 o... [18:26:43] (03PS2) 10Bartosz Dziewoński: Remove some dead CSS [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260386 [18:26:45] (03PS1) 10Bartosz Dziewoński: Prettify code creating titles in predefined namespaces [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260616 [18:26:47] (03PS1) 10Bartosz Dziewoński: Correct mixups of default vs predefined namespaces when creating titles [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260617 [18:30:11] (03CR) 10jenkins-bot: [V: 04-1] Correct mixups of default vs predefined namespaces when creating titles [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260617 (owner: 10Bartosz Dziewoński) [18:30:30] 6Multimedia, 10MediaWiki-General-or-Unknown, 10VisualEditor, 10VisualEditor-MediaWiki-Media, 5Patch-For-Review: Show determinate progress bar for the image upload in mw.ForeignStructuredUpload.BookletLayout - https://phabricator.wikimedia.org/T115861#1898775 (10matmarex) I actually like the design. I don... [18:32:10] (03CR) 10jenkins-bot: [V: 04-1] Prettify code creating titles in predefined namespaces [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260616 (owner: 10Bartosz Dziewoński) [18:33:47] lindseyanne: argh, i forgot about our meeting D: if you have time now, we could chat [18:40:56] one way to replace year/month menu with oojs ui may be to use client-side JS to "hide" the fields and add a date widget [18:41:18] and update the legacy fields when a date is chosen with the widget [18:44:53] also keeps non-js users happy [18:46:06] omg my day extension of ReverseChronPager works [18:46:24] 6Multimedia, 10MediaWiki-General-or-Unknown, 10VisualEditor, 10VisualEditor-MediaWiki-Media, 5Patch-For-Review: Show determinate progress bar for the image upload in mw.ForeignStructuredUpload.BookletLayout - https://phabricator.wikimedia.org/T115861#1898823 (10Esanders) Another issue is introducing yet... [18:47:37] 6Multimedia, 6Commons, 10MediaWiki-File-management, 7Design: Add a way to stop animated GIFs - https://phabricator.wikimedia.org/T85838#1898827 (10Whatamidoing-WMF) @Tgr I think you're looking for a "perfect" solution, and I'm aiming for "done". Giving me a button the covers up the blinking/distracting/an... [18:49:59] OK I've spoken with lindseyanne, if MatmaRex wants to jump in that's cool [18:50:13] But I have to run to the store and then probably clean my apartment quickly before leaving for Las Vegas [18:50:22] So y'all be good now [18:50:37] MatmaRex : now works :) [18:51:28] same link as original [18:51:41] (which is not the one marktraceur was on ;) ) [19:02:50] Hydronium: :o [19:32:30] (03CR) 10Gergő Tisza: [C: 04-1] "This is definitely not going to work as you are not setting the gender property anywhere." [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) (owner: 10MtDu) [19:38:38] just some logic bugs I have to weed out, but i can filter by day :o [19:38:56] actually wasn't that difficult. now all the special pages need to be modified [19:39:29] Does there need to be another class that filters in a range (i.e. two dates?) [20:07:43] tgr: Where would you recommend setting the gender propery? ImageInfo, or adding it as a new parameter as I did in Patch 16? [20:08:22] MtDu: do you have the chat logs from yesterday? [20:08:32] I can download them. [20:08:44] we talked about it quite a bit [20:09:35] I can just recap if you don't have them at hand - you should create a gender property in ImageInfo and set it in mmv.js in the metadataPromise.done handler [20:10:53] tgr: Ok [20:17:24] tgr: So set the gender in mmv.model.Image.js? [20:17:35] ImageInfo [20:18:59] tgr: In the file mmv.provider.ImageInfo.js? [20:19:40] hm, no, you are right [20:19:46] the model is called Image [20:20:03] one of those stupid inconsistencies we never got around to clean up [20:21:13] tgr: This is what I did. https://dpaste.de/Tn3O#L25,104,105 [20:22:29] you shouldn't add an extra parameter, you are never going to pass anything there [20:22:46] tgr: ok [20:22:48] just create the property so that it can be used elsewhere [20:24:09] 6Multimedia, 10UploadWizard, 5Patch-For-Review: Rewrite UploadWizard with mustache templates - https://phabricator.wikimedia.org/T96520#1899073 (10Jdlrobson) [20:30:15] tgr: My qunit test is not running. http://127.0.0.1:8080/wiki/Special:JavaScriptTest/qunit/plain# It was just running a few minutes ago. [20:30:46] I can't see that URL, it's accessible to your machine only [20:31:06] tgr: I know. Is that the right url? [20:31:23] it looks right [20:31:43] if your vagrant is configured to be on port 8080, which is the default [20:31:50] is the VM running? [20:32:03] tgr: yep. The tests ran just 5min. ago [20:32:53] what kind of error are you getting? [20:33:26] tgr: I don't see one. [20:33:36] tgr: Is this how you set the gender? https://dpaste.de/bWyD [20:33:46] tgr: are we using the null I set in the other file here? [20:34:02] tgr: I don't get which one we are setting using which one. If that makes sense. [20:34:57] userInfo.gender is set by mmv.provider.UserInfo [20:35:17] that's the class that does the API request to fetch user data [20:35:48] you added a new property to the imageinfo object, so that's the one you'll need to set [20:37:46] tgr: So I need to add that to the params at the top as well right? [20:37:56] tgr: https://dpaste.de/Tn3O#L25 [20:39:35] tgr: So it doesn't return a gender not defined error? [20:39:51] tgr: This is what I have in addition to the paste I sent you earlier. https://dpaste.de/4RkE#L82 [20:40:36] that looks correct [20:40:59] tgr: So I do add it here? https://dpaste.de/Tn3O#L25 [20:41:27] IIRC the other problem with your patch is that it is using info.gender instead of info.imageInfo.gender [20:41:44] where info is a Lightboximage object or some such [20:42:07] no, why would you add it there? [20:42:35] tgr: Because I added it at the bottom? Don't I have to add it to the top? Or is it just always null? [20:42:52] tgr: And this is the new mmv.EmbedFileFormatter.js getCreditHtml https://dpaste.de/Cfgd [20:42:55] you are never calling new Image(........, gender) so there is no point in declaring it [20:43:19] tgr: Ok. [20:43:43] in Javascript it is customary to declare fields by setting them to null in the constructor [20:43:58] that's unrelated to what parameters the constructor has [20:44:20] tgr: So is that all I need to do? [20:44:23] tgr: That makes sense. [20:44:35] for models the constructor parameters and the object properties tend to be more or less the same, but it's not a requirement [20:45:44] tgr: I can't run the qunit tests for some reason. [20:45:54] using Image.gender is a somewhat hacky solution but the non-hacky solution would be more complex and not really worth the effort [20:46:20] tgr: Yeah. [20:46:34] the new EmbedFileFormatter looks good as well [20:46:48] what kind of test error are you getting? [20:46:50] tgr: So, ideally, from just the things I changed, it shouldn't cause the qunit tests to fail? [20:47:05] tgr: I don't get an error. I just don't get anything. It comes up a blank page. [20:47:22] tgr: Yesterday, this was fixed by adding a debug = true somewhere? Does that ring a bell? [20:47:53] it might break the EmbedFileFormatter tests since they do not set the gender but it definitely should not break half a dozen unrelated tests [20:48:17] like DownloadPanel.set(), or what was the one you had trouble with the last time [20:48:25] tgr: Ok. [20:48:40] debug=true makes your stack traces nice [20:48:54] it probably won't affect whether your tests run at all [20:49:04] you should look at the logfiles [20:49:07] tgr: I think the reason my tests aren't runnign is because my brother is playing an online game that might be using the port? [20:49:13] tgr: So should I just push a patch and see? [20:49:23] logs/mediawiki-exception-something in the vagrant folder [20:49:28] that works too [20:49:48] tgr: Should I take a look at the embedfileformatter.test file before I push it or not? [20:50:03] I don't think port 8080 is used by anything but I guess it's possible [20:50:45] yeah, you should probably set the gender in the tests [20:51:05] tgr: to null right? [20:52:31] tgr: In getcredittext and getcredithtml? [20:54:15] gender is one of 'male', 'female', 'unknown' [20:54:20] tgr: In those lines. https://dpaste.de/AHrs#L7,198,214,242,259 [20:54:24] so to one of those [20:54:43] tgr: Just in those right? [20:54:50] it won't really matter since tests run in English which does not depend on the gender [20:55:16] tgr: So I don't need to mess with that file? [20:56:24] you don't need to change createEmbedFileInfo, that's not used by the tests that check credit message generation [20:57:02] tgr: So is there nothing to change? [20:57:39] uh, wait, I'm misreading that [20:58:17] you'll need to handle options.gender in createEmbedFileInfo [20:58:32] Ok. [20:58:39] and set it in all the imageInfo: {...} object declarations [20:59:32] tgr: Ok. So in the other five highlights? [20:59:46] tgr: And where is that file that I can change imageInfo? [21:00:11] four by my count [21:00:26] I don't understand your last question [21:00:47] tgr: What file can I handle options.gender? [21:00:58] tgr: Is it in the qunit tests folder? [21:02:38] tgr: Oh I found it. It's in the file I'm looking at. *facepalm [21:03:07] I was referring to the options parameter of createEmbedFileInfo [21:05:17] tgr: So this is the function right? https://dpaste.de/AHrs#L4,5,6,7,8,9,10,11,12,13,14,198,214,242,259 Am I supposed to add options.gender? [21:10:14] tgr: so I have to find each instance of the createEmbedFileInfo [21:10:35] tgr: And add gender? [21:11:05] MtDu_: you need to do two things [21:11:21] tgr: I think I got it. Let me send you it. [21:11:39] 1) change createEmbedFileInfo in such a way that you can pass a gender option to to it and it will set imageInfo.gender [21:12:31] 2) find the createEmbedFileInfo calls where the test will care about gender, which is the getCreditText and getCreditHtml tests [21:12:47] and do pass the gender to createEmbedFileInfo there [21:14:35] 6Multimedia, 6Commons, 10MediaWiki-File-management, 6Performance-Team: Use Chroma subsampling for image thumbnails (save 17% bandwidth and save memory on mobile) - https://phabricator.wikimedia.org/T122242#1899257 (10Krinkle) 3NEW [21:15:02] tgr: Is this what it should be? https://dpaste.de/jg6G#L9,203,204,225,226,249,250,272,273 [21:15:58] almost, but Image() does not have a gender parameter, remember? [21:20:05] tgr: So just don't add that there? [21:20:27] just set it the same way you set it in mmv.js [21:21:06] tgr: Gotcha. [21:21:56] tgr: https://dpaste.de/FmKL#L12? Is the spacing right? [21:27:09] tgr:^ [21:27:40] what's userInfo? [21:28:18] tgr: Oh wait sorry. [21:28:39] tgr: https://dpaste.de/NW4y#L12 [21:29:33] Hydronium: Love that chemistry! [21:29:42] :) [21:29:50] tgr:^ [21:29:55] MtDu_: that works although in that case there is no point in declaring the gender in all the other places in the test [21:29:58] too bad you can't have plus signs in IRC nicks [21:30:26] tgr: So in that function, I don't need to declare it? Or does it need to be null? [21:31:03] OH-: you can probably use U+2795 [21:31:15] if you don't mind that half the people see a tofu box [21:32:00] tgr:^ [21:32:20] seems as if freenode doesn't support UTF in nicks :/ but thanks for the suggestion [21:32:24] tgr: Oh wait. you mean if I set it there. I don't need to set it elsewhere. [21:32:31] tgr: Right? [21:32:44] MtDu_: the point of the other changes you did in that file is to pass gender as part of the options parameter of createEmbedFileInfo, right? [21:32:59] tgr: Yep. But now that I set it already, there's no point. [21:33:00] so I was thinking of copying that to imageInfo [21:33:17] but if you don't pass it at all and set it to a constant value, that works too [21:34:05] tgr: So like this? https://dpaste.de/Ccyw [21:34:25] OH-: use ÷ then and hope no one notices the gaps :) [21:35:10] MtDu_: yep [21:35:16] tgr: Let me push a patch now. [21:35:17] aw, freenode won't let me do that either. oh well [21:39:08] (03PS18) 10MtDu: Add gender support to Multimediaviewer-text-embed-credit-text-tbls and other messages [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) [21:42:02] (03CR) 10jenkins-bot: [V: 04-1] Add gender support to Multimediaviewer-text-embed-credit-text-tbls and other messages [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) (owner: 10MtDu) [21:44:36] (03PS19) 10MtDu: Add gender support to Multimediaviewer-text-embed-credit-text-tbls and other messages [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) [21:47:04] (03CR) 10jenkins-bot: [V: 04-1] Add gender support to Multimediaviewer-text-embed-credit-text-tbls and other messages [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) (owner: 10MtDu) [21:49:53] tgr: I really wish my qunit tests were working...... [21:50:18] MtDu_: did you check your logs? [21:50:27] tgr: Which logs? [21:50:42] apache, hhvm, mediawiki-exception are the more useful ones [21:51:39] alternatively you can do https://www.mediawiki.org/wiki/Manual:JavaScript_unit_testing#From_the_command_line [21:52:41] tgr: This is the hhvm error. https://dpaste.de/MZQp [21:56:08] tgr: Let me restart the computer. [22:09:36] tgr: I'm getting a lot more errors then I expected. Not just in mmv. mmv.logging.PerformanceLogger, mmv.logging.ActionLogger, mmv.logging.AttributionLogger, mmv.logging.DimensionLogger, mmv.logging.ViewLogger [22:09:50] tgr: And mmv.model [22:10:31] that sounds like there is a syntax error somewhere and e.g. creating an Image model throws an exception [22:10:52] if you have set up jshint locally, it might give you a clue [22:11:28] tgr: Look here. https://gerrit.wikimedia.org/r/#/c/260221/17..19/resources/mmv/provider/mmv.provider.ImageInfo.js What is that red? [22:11:44] red means a whitespace problem [22:11:52] trailing whitespace, usually [22:12:28] oh, you meant you are getting code convention erros, not unit test errors [22:12:43] you might be using a text editor that messes up the whitespace [22:13:10] tgr: Ok. But all my tests pass except for qunit. [22:15:48] push the change then and I'll look at the errors [22:15:57] tgr: It's patch 19 [22:17:16] wow [22:17:18] tgr: I just fetched patch 19. Only 6 failed. :) [22:17:22] that's a lot of test failures [22:18:00] the needGender one seems like a genuine error [22:18:14] tgr: Don't look at my previous message listing those errors. [22:18:49] tgr: The only errors I see are related to. https://dpaste.de/ff8i [22:19:03] tgr: cannot read gender property of undefined. [22:19:07] I would say most others are fallout from needGender having some kind of syntax error [22:20:05] tgr: The function itself? [22:20:13] seems so [22:20:37] tgr: Oh. I think I know what it is. Should I be pushing unspecified instead of unknown? [22:20:52] tgr: https://dpaste.de/VvVz [22:20:59] no, that would not cause an error [22:21:34] or maybe it would? I haven't looked at the mw.message code closely [22:21:56] but I think anything thats not 'male' or 'female' is taken to mean unknown [22:23:10] tgr: hm. you're right. [22:23:29] anyway the error is "ReferenceError: True is not defined" [22:24:38] I have no idea where that comes from [22:24:43] I'll have to test that locally [22:24:51] in an hour, maybe [22:25:14] til then you could fix the whitespace and permission problems [22:25:41] tgr: What permission problems? [22:26:09] doc/generate and that other file whose name I forgot [22:26:40] tgr: What's that command you told me to run? [22:26:48] chmod, probably [22:27:01] tgr: And can you tell me what's wrong with the spacing? [22:27:06] tgr: https://dpaste.de/tnKD [22:27:12] tgr: I can't tell in my editor. [22:27:40] empty lines should be properly empty [22:27:46] not contain any whitespace or tab [22:28:45] tgr: I got it. [22:29:14] tgr: Could you run the tests and tell me what I need to fix via gerrit comments? I will be out for like 4 hours. [22:29:57] ok [22:30:15] tgr: Again, thank you for your patience. I think I've learned not only a lot about writing code, but also about how to approach a task. I need to be less rushed and just face it. [22:30:45] tgr: Anyways, I'll push a patch fixing those two things mentioned above, and wait on your comments. Thanks! :) [22:33:14] tgr: Are these the right commands? https://dpaste.de/tFFZ [22:34:19] tgr:^ [22:37:19] tgr: Because I have run those before. [22:39:44] MtDu: those look correct [22:39:45] tgr:nvm. It worked [22:39:54] are you working on a Windows machine? [22:39:54] (03PS20) 10MtDu: Add gender support to Multimediaviewer-text-embed-credit-text-tbls and other messages [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) [22:40:09] tgr: That one should fix those two error.s [22:40:09] Windows can mess up file permissions sometimes [22:40:19] It didn't work [22:40:26] how about --chmod=755 [22:41:33] try running "git checkout -- docs/generate importml.sh" after those two commands [22:41:36] It says it changed the mode to 100644 when I committed, but it didn't show up [22:42:00] it should change to 100755 [22:42:02] And yes, I'm using a windows machine to push patches [22:42:22] (03CR) 10jenkins-bot: [V: 04-1] Add gender support to Multimediaviewer-text-embed-credit-text-tbls and other messages [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) (owner: 10MtDu) [22:42:41] what's happenind is probably that you are updating the index but then committing with -a which updates the index based on the filesystem again and reverts things back [22:42:51] that or Windows being weird [22:43:09] tgr: I do git commit --all --amend, then git remote update, then git review -y [22:43:31] yeah, that's probably the problem [22:43:44] doing the git checkout should fix that [22:44:18] tgr: Ok. So run the two commands then git commit --all --amend then git checkout (command you gave) then git remote update, then git review -y [22:45:13] no, checkout before commit [22:49:59] tgr: It still doesn't work. It still has it at 100644 [22:50:25] MtDu: let me test locally [22:50:38] after lunch [22:50:45] tgr: Ok. I will be back in 4 hours. [22:50:50] tgr: Will talk to you then [22:50:57] tgr: Actually, about 5 hrs. [22:51:13] tgr: Enjoy lunch [23:57:22] (03CR) 10Gergő Tisza: "Re: file permissions, this worked fine for me:" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) (owner: 10MtDu)