[00:00:24] hm, the patch still removes the executable bit for those two files [00:01:12] tgr: I don't understand what I need to change in unit test. [00:02:33] yeah, our unit test infrastructure is not exactly user-friendly [00:02:53] you can look at https://integration.wikimedia.org/ci/job/mwext-qunit/9273/consoleFull [00:03:25] and find the Expected:... Actual:... bit [00:04:04] but of course the stack trace is completely useless due to minification [00:04:21] so you'll have to run the tests yourself in debug mode to get a usable stack trace [00:04:40] that tells you which line of the test needs fixing [00:05:42] MtDu: re: file permission changes, it's probably easiest if you just fix them by hand [00:05:56] chmod a+x [00:06:01] then add/amend/push [00:12:08] tgr: Can you walk me through the debugging and finding which line I need to change? [00:13:43] https://www.mediawiki.org/wiki/Manual:JavaScript_unit_testing#Run_the_tests explains how to run the tests [00:14:18] there is a module filter dropdown, you might want to set that to the mmv module, otherwise they'll run for a while [00:14:36] Ensure $wgEnableJavaScriptTest is set to true.. [00:14:40] and there is a debug mode checkbox which you'll need to enable [00:14:41] What file is that in? [00:14:59] if you are using vagrant, that's going to be set correctly [00:15:32] $wg stuff is usually in LocalSettings.php, although vagrant has a more complicated setup [00:15:45] but also comes with most things configured the way you need them [00:15:53] I'm getting a 503 service error [00:15:59] When I try to go the the home page on 127 [00:21:30] Do you mind running them on your browser? I'm not sure why my vagrant is not working. [00:22:39] tgr: Do you mind running them on your browser? I'm not sure why my vagrant is not working. [00:24:17] you will need vagrant to test that your patch is working, anyway [00:24:23] what's wrong with it? [00:24:30] I get a 503 service error [00:24:33] It was working earlier. [00:24:42] When I tested the other stuff. [00:24:51] I don't know why it's giving me that. [00:25:49] is it running? [00:26:00] you can check with vagrant status [00:26:05] Yeah. [00:26:25] hm [00:26:38] 503 is not an error code I would expect from vagrant [00:27:06] well, first step of problem solving, plug it off and back in :) [00:27:16] which in this case is done by vagrant reload [00:27:43] Am I doing ok with this task? I feel like I'm taking forever. [00:29:34] is it your first mediawiki patch? figuring out vagrant, gerrit and so on is not easy the first time [00:29:56] No [00:30:11] But this is on a new machine [00:30:48] it seems like some of your problems are from not doing local tests [00:31:24] in general, doing tests locally is more efficient because the test runner on gerrit is 1) slow 2) gives barely useful output [00:31:50] I got the 502 error [00:31:53] again [00:31:56] 503* [00:31:57] you waste much less time if you try to spot the error in a local qunit run than in the gerrit/jenkins log [00:33:22] try "vagrant ssh" to get into the vagrant box, then "curl https://localhost/wiki/Main_Page" [00:33:28] is that still an error? [00:35:03] how do you connect to vagrant, anyway? open 127.0.0.1:8080 in your browser? [00:35:14] Yeah. [00:35:51] It says theres like 20 updates, and to run git pull --rebase. [00:36:06] In my vagrant dir if I want them. [00:36:21] In my vagrant dir if I want them. [00:36:33] Do I need to update those? [00:36:36] there is no need to keep vagrant up to date [00:36:53] curl: (7) Failed to connect to localhost port 443: Connection refused [00:36:53] if it's half year old, that's maybe a problem [00:37:10] if it's a week or too old, that doesn't matter [00:37:26] sorry, the curl target should have been http:, not https:, my bad [00:38:22] https://dpaste.de/4KX5#L [00:38:25] Still get the error. [00:39:02] weird [00:39:39] the apache logs are either at /vagrant/logs or at /var/log (on your guest machine), can you check them? [00:39:54] What do you want me to check? [00:41:19] tail -n 30 /vagrant/logs/apache2.log [00:47:07] https://dpaste.de/AOL3 [00:48:31] tgr: https://dpaste.de/AOL3 [00:49:48] ok, can you do the same for the hhvm log? [00:49:50] Are those the ones you wanted? [00:50:35] on second thought the mediawiki error log [00:50:58] that was the one, yes, but all it tells is that the web server cannot connect to the php process [00:51:30] tail -n30 /vagrant/logs/mediawiki-exception.log [00:51:43] vagrant@mediawiki-vagrant:~$ tail -n30 /vagrant/logs/hhvm/error.log [00:51:50] can you try those? [00:54:00] How do I get out of vagrant@mediawiki-vagrant:~$ [00:54:36] ctrl-d (but you don't need to, for those two) [00:55:36] https://dpaste.de/Ni8p [00:56:23] and https://dpaste.de/1CwB [01:03:30] ah, looks like you'll need to update [01:04:05] you probably did a git pull on mediawiki core, but did not run the upgrade script which is needed when git pull brings in a DB change [01:04:23] the easiest way is using the "vagrant git-update" command [01:04:26] MtDu: ^ [01:04:47] Do this in vagrant? [01:05:11] Like while cded in vagrant [01:05:14] tgr:^ [01:05:21] no [01:05:41] all commands that start with "vagrant" must be done on your host machine [01:05:46] where vagrant is installed [01:05:51] so in vagrant ssh? [01:06:04] no, host machine is your normal computer [01:06:28] you use vagrant ssh to get into the guest (virtual) machine [01:26:43] okay [01:30:55] Btw, it's not updating the stuff that I have changes in [01:34:47] yes, there is no way for git to figure out how to safely do that [01:35:03] but that's fine as long as core is updated [02:09:35] tgr: Ok. I updated everything. I got this at the end. https://dpaste.de/AHNW [02:09:40] tgr: What do I do now? [02:11:59] tgr: I still get the 503 error. [02:12:14] MtDu: you need to update core [02:12:22] tgr: What command? [02:12:26] that did not happen this time because it was on a branch [02:12:37] you can return to master with git checkout master [02:12:48] (in the mediawiki directory) [02:13:00] after that, run vagrant git-update again [02:13:28] you can reset the other extensions to master as well if you don't need them anymore but it doesn't really matter [02:13:33] wait you want me to cd vagrant cd mediawiki and then git checkout master and then git update? [02:14:07] tgr:^ [02:18:51] MtDu: yes, except the last one is vagrant git-update [02:19:23] vagrant git-update does a number of things - a git pull on all your repos, a DB schema update on all your extensions, it pulls new libraries via Composer [02:46:34] tgr: I still get a 503 service error. :( [02:47:35] tgr: https://dpaste.de/vdRr [02:48:45] well, looks like there is something wrong with the file it mentions [02:49:17] maybe you edited it and there was a merge conflict or something and the file became invalid? [02:52:40] tgr: I don't see what's wrong with my file. [02:53:35] well, you can paste it into some JSON validator and it will tell you [04:20:10] (03PS14) 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:31:29] tgr: Is my new patch okay? [04:33:05] tgr: I took the GENDER stuff out of the en.json file. Wait. I'm supposed to leave that in right? [04:33:36] tgr: I am now on my mac, and am able to access http://127.0.0.1:8080/wiki/Special:JavaScriptTest/qunit/plain#. I have put the GENDER back into the en.json file. What do I do now? [04:36:19] tgr: I ran the tests. [04:37:55] tgr: I see the errors. They are here. https://dpaste.de/3vd8 [04:40:10] tgr: I just changed it what GENDER stuff it should have had based on the message. Is that right? [04:44:23] MtDu__: yeah, you should leave GENDER in en.json [04:44:39] tgr: I ran the tests in my mac witht he updated tests. They worked!!!!!!! [04:44:52] tgr: But I think I'm having some issues pushing a git review [04:46:20] MtDu__: are you using git review? [04:46:26] yes. [04:46:34] tgr: https://dpaste.de/S0cF [04:46:40] do you see a huge list of unfamiliar changes [04:46:41] , [04:46:43] ? [04:46:47] tgr: Lemme go ahead and copy the files to my windows machine and push the patch there. [04:47:30] seems like a mistyped user name or password [04:47:46] you can set up an ssh key for gerrit, that makes life easier [04:47:52] tgr: It's not that. I think it is something with my git config file. [04:48:17] well, it's an authentication error from gerrit [04:48:48] Not sure. I don't see one in MMV. That's why I switched to my Windows machine to push updates for this task. I couldn't get it working on my mac. It's really inconvenient, because my mac I can run 127 and test stuff, but can't push updates, but it's vice versa on my Window smachine. [04:49:08] tgr: Well, it's not that. Because git review should only ask you for your ssh password right? [04:49:22] depends [04:49:45] gerrit can use HTTP authentication or SSH authentication [04:50:00] in the first case it will ask for the same password as the gerrit web interface [04:50:28] in the second case your ssh client will ask for the passphrase of your private key (if it has one) [04:50:46] it depends on how your remote is configured [04:51:22] git remote show gerrit -> whether it starts with https:// or ssh:// [04:51:45] (03PS15) 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:52:18] tgr: Ok. [04:52:49] tgr: https://dpaste.de/jX3T [04:53:34] tgr: It's ssh. So that means something else is wrong. Do you know what? [04:53:39] so if you prefer SSH authentication, you need to change that [04:53:51] ah, sorry, wasn't looking properly [04:54:07] tgr: So what is wrong with my ssh? [04:54:14] tgr: Also, take a look. https://gerrit.wikimedia.org/r/#/c/260221/ :) [04:54:27] the other option is that git-review is configured to use some other remote (origin, probably) [04:55:47] try specifying it manually with git review -r gerrit ... [04:55:54] and see if that has an effect [04:56:29] tgr: that won't push a patch will it? [04:56:54] depends on the ... part [04:57:22] you can use the --dry-run option to make sure it does not [04:57:37] tgr: If I don't want it to push a patch, what do I type? [04:58:05] git review --dry-run -r gerrit [04:58:05] tgr: Also, can I submit my task on GCI now? [04:59:10] hm [04:59:19] that unit test seems wrong [04:59:33] it should have just worked as is [04:59:40] not sure what is going on there [05:00:06] tgr: It's just that now that I have added GENDER to the en.json file, it's different from what it used to be. [05:00:45] it's not really, the test parses the message to its English languge version, and GENDER should disappear in the process [05:01:14] maybe you are not passing the fourth parameter, and that's why GENDER:$4 is left as is [05:02:48] you should try to set lastUploader in the test [05:02:48] Aren't I pushing the parameter in line 32? [05:03:13] so the unit tests are testing a single function [05:03:25] in this case getCreditHtml [05:03:30] tgr: So add in lastUploader and make it the same as 'Author'? [05:03:37] everything else is pretty much faked up [05:04:04] you can see that the getCreditHtml in the test is passed a hand-made imageinfo object [05:04:14] and that object does not have lastUploader set [05:04:58] the nice thing to do is probably to fix that in getCreditHtml - if lastUploader is not available, just use 'unknown' instead [05:05:10] tgr: Ok. I changed the test to this. https://dpaste.de/91nY And it failed. It's not getting rid of the GENDER [05:05:11] or 'unspecified' or whatever we use for gender [05:06:04] what if you set lastUploader to 'male'? [05:06:34] tgr: It fails. [05:06:53] tgr: https://dpaste.de/ginx [05:07:23] weird [05:07:30] give me a sec to test that locally [05:07:36] tgr: Ok [05:09:56] tgr: perhaps is something wrong with the if statement in mmv.EmbedFileFormatter? [05:10:34] uh, yeah [05:10:41] indeed [05:10:54] I even wanted to mention that at some point then totally forgot [05:11:11] there should be an else branch there where you push 'unknown' [05:12:45] tgr: Ok. https://dpaste.de/psax Is that right? With the spaces and indents? [05:13:30] usually we write it like [05:13:39] } else { [05:13:55] otherwise looks right [05:13:57] tgr: Ok. [05:14:13] tgr: It still fails though. Both with lastUploader set as 'male' and set as 'Author'. [05:22:25] tgr: Here is the mmv.EmbedFileFormatter.js file https://dpaste.de/EV9H and the mmv.EmbedFileFormatter.test.js file. https://dpaste.de/1rMw [05:36:29] tgr: I'm not sure why it still isn't getting rid of the GENDER. [05:37:22] mw.message has lots of black magic in it [05:37:43] works differently depending on what JS classes are loaded, probably related to that [05:38:05] I'll walk through it with a debugger, I'll just need time to set up my environment [05:38:27] tgr: No problem. [05:39:28] tgr: Thanks for helping me. I've learned a lot about running tests locally. :) [05:47:43] (03PS1) 10Ananay: Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead in UploadWizard (Wikimedia) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 [05:48:50] https://gerrit.wikimedia.org/r/#/c/260221/15/resources/mmv/mmv.EmbedFileFormatter.js You're using spaces for indents, which is why it's all screwed up [05:52:50] Hydronium: Fixed. [06:08:06] tgr: Any updates? [06:15:47] tgr: Is there anything you want me to do? [06:22:21] tgr: Can you post anything you find as comments on the gerrit page? I need to go sleep. It's 12:30 in the morning. [06:23:07] MtDu__: us, sorry, I need to do some cleaning on my laptop since vagrant is being sluggish [06:23:35] I will review the gerrit patch when that's done [06:24:04] tgr: The current gerrit patch is not right. [06:24:39] test it and see if I can figure out what's wrong with GENDER, I mean [06:24:41] tgr: All I did was change the unit test to have the GENDER in it, so it would pass. But you can still comment on it. [06:24:51] tgr: Oh ok. [06:25:13] tgr: Keep me posted then. Night! [06:25:27] night [08:29:39] (03CR) 10Gergő Tisza: "Okay, so the problem is a lot simpler than I assumed. getCreditText(), which is the function you changed, passes its tests (so GENDER does" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) (owner: 10MtDu) [08:38:01] (03CR) 10Gergő Tisza: [C: 04-1] Add gender support to Multimediaviewer-text-embed-credit-text-tbls and other messages (036 comments) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) (owner: 10MtDu) [08:55:57] (03CR) 10Gergő Tisza: "Tested by changing the English message (via the MediaWiki:Multimediaviewer-text-embed-credit-text-tbls-nonfree wikipage) to be gender-depe" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) (owner: 10MtDu) [09:05:29] (03CR) 10Gergő Tisza: "I don't think passing the username is enough in Javascript - that only works in PHP. In JS either an mw.User object is needed (which is no" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/260221 (https://phabricator.wikimedia.org/T85685) (owner: 10MtDu) [12:57:59] MatmaRex: o/ [14:07:31] hi prtksxna [14:18:50] (03CR) 10Bartosz Dziewoński: [C: 04-1] "You only did half of each of the two pieces of work here." [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (owner: 10Ananay) [14:21:39] 6Multimedia, 10UploadWizard, 7Easy, 3Google-Code-In-2015, and 2 others: Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead - https://phabricator.wikimedia.org/T120627#1894730 (10matmarex) https://gerrit.wikimedia.org/r/260327 [14:31:55] (03PS2) 10Ananay: Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead in UploadWizard (Wikimedia) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 [14:34:13] (03CR) 10Aklapper: "https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines - this is missing the reference to the Phabricator task and the lines are " [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (owner: 10Ananay) [14:34:43] (03CR) 10jenkins-bot: [V: 04-1] Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead in UploadWizard (Wikimedia) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (owner: 10Ananay) [14:37:52] (03PS3) 10Ananay: Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead in UploadWizard (Wikimedia) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 [14:41:28] (03CR) 10jenkins-bot: [V: 04-1] Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead in UploadWizard (Wikimedia) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (owner: 10Ananay) [14:51:09] (03PS4) 10Ananay: Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead in UploadWizard (Wikimedia) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (https://phabricator.wikimedia.org/T120627) [14:53:49] (03CR) 10jenkins-bot: [V: 04-1] Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead in UploadWizard (Wikimedia) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (https://phabricator.wikimedia.org/T120627) (owner: 10Ananay) [14:56:52] (03CR) 10Bartosz Dziewoński: Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead in UploadWizard (Wikimedia) (032 comments) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (https://phabricator.wikimedia.org/T120627) (owner: 10Ananay) [15:06:02] (03PS5) 10Ananay: Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (https://phabricator.wikimedia.org/T120627) [15:06:27] Such noise [15:07:51] (03CR) 10Bartosz Dziewoński: [C: 032] Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (https://phabricator.wikimedia.org/T120627) (owner: 10Ananay) [15:09:04] poor marktraceur [15:09:06] 6Multimedia, 6Commons: A new panoramic viewer for commons - https://phabricator.wikimedia.org/T105789#1894815 (10MarkTraceur) p:5Triage>3Normal [15:09:23] I'm good. I'm going to make noise as revenge. [15:11:48] (03Merged) 10jenkins-bot: Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260327 (https://phabricator.wikimedia.org/T120627) (owner: 10Ananay) [15:12:21] (03PS4) 10MarkTraceur: Kill polling for moveFileInputToCover [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260059 (https://phabricator.wikimedia.org/T121901) [15:12:43] MatmaRex: My bad, I left a 250 in from when I was debouncing at those lines instead of at the function definition [15:13:18] 6Multimedia, 10UploadWizard, 7Easy, 3Google-Code-In-2015, 7Technical-Debt: Remove JS adding/removing 'hover' CSS class, use :hover pseudoclass instead - https://phabricator.wikimedia.org/T120627#1894830 (10matmarex) 5Open>3Resolved a:3ananayarora [15:13:19] (03CR) 10MarkTraceur: Kill polling for moveFileInputToCover (031 comment) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260059 (https://phabricator.wikimedia.org/T121901) (owner: 10MarkTraceur) [15:14:23] 6Multimedia, 6Commons, 10MediaWiki-Uploading: Commons private-to-public setting - https://phabricator.wikimedia.org/T111792#1894837 (10MarkTraceur) p:5Triage>3Normal Yeah, how is this different from the stash? [15:14:41] Argh I made it worse [15:14:54] Now every wikibugs message pings me and I get noise on my computer and on my phone [15:15:02] I regret everything [15:21:03] 6Multimedia, 6Commons, 10Wikimedia-General-or-Unknown: Cryptic "413 Request Entity Too Large" error when uploading new version of >100MB file on Commons - https://phabricator.wikimedia.org/T115984#1894857 (10MarkTraceur) p:5Triage>3Normal [15:21:05] Maybe I should unignore wikibugs and gerrit. I miss out on so much fun. [15:22:07] 6Multimedia, 10MediaWiki-Uploading: Special:Upload shouldn't try stashing files for anonymous users - https://phabricator.wikimedia.org/T115822#1894859 (10MarkTraceur) p:5Triage>3Normal [15:22:46] prtksxna: I had a good setup when I had them ignored in team channels, but I was also in -feed [15:23:13] marktraceur: I don't know how to setup per channel ignore on this client. [15:23:27] That's why I'm stuck in the dark pit of despair also [15:23:44] Accursed newfangled convenient technology [15:25:10] Is this even a client specific thing? Shouldn't I be able to type the right command? [15:26:45] No, ignore is a client-specific implementation detail [15:26:56] It's not the server not sending you messages from wikibugs, it's your client discarding them [15:27:30] Or not displaying them, as the case may be [15:28:49] marktraceur: oh :( [15:32:06] mw.Upload.Dialog is broken because of mw.Upload.BookletLayout :] [15:32:08] * :\ [15:32:30] prtksxna: We got a bug for that? [15:32:58] marktraceur: I don't think so [15:32:59] * prtksxna looks [15:33:26] I mean, it's either that or you just found something and nobody else has noticed [15:33:36] But I guess what I'm really asking is, how is it broken? [15:34:01] tgr_: https://dpaste.de/OerL#L363 Are you talking about line 363? [15:34:19] marktraceur: Calling something that doesn't exist, possibly because of a typo https://gerrit.wikimedia.org/r/#/c/260384/ [15:35:02] Ooh, I hate it when that happens [15:35:05] That was probably me [15:35:35] Merging [15:36:44] Thanks :) [15:37:17] I wonder how we could get notifications about cross-wiki upload stuff in here [15:40:32] tgr_: https://dpaste.de/OerL#L363 Are you talking about line 363? Or are you talking about the actual function? [15:41:08] MtDu: Hey, are you still talking about the same patch? [15:41:25] +marktraceur: Yep. Look at tgrs last comment, I don't quite understand it. [15:42:40] MtDu: You need to get the gender value out of userInfo in that callback, and somehow keep it so your code can use it [15:43:26] +marktraceur: Could you explain a bit more? [15:43:37] MtDu: You understand what the problem is, right? [15:44:03] +marktraceur: GENDER is not working in getcredithtml. Because the lastUploader isnever passed? [15:45:13] MtDu: GENDER won't work with lastUploader in JavaScript because the JS library that parses messages doesn't support GENDER with usernames. [15:45:40] +marktraceur: So I need to get the actual gender. [15:46:07] (03CR) 10Bartosz Dziewoński: [C: 032] Kill polling for moveFileInputToCover [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260059 (https://phabricator.wikimedia.org/T121901) (owner: 10MarkTraceur) [15:46:45] MtDu: Right, which is in the userInfo object at the line you linked to [15:47:00] So you need to save it somewhere and use it for the GENDER call. [15:48:13] (03PS1) 10Bartosz Dziewoński: Remove some dead CSS [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260386 [15:48:23] So where is it in this line? viewer.ui.panel.setImageInfo( image, imageInfo, repoInfo, userInfo ); [15:49:21] (03Merged) 10jenkins-bot: Kill polling for moveFileInputToCover [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/260059 (https://phabricator.wikimedia.org/T121901) (owner: 10MarkTraceur) [15:50:05] MtDu: "[it] is in the userInfo object" --marktraceur five minutes ago [15:51:31] prtksxna, MatmaRex, looks like we've got a ball game for the standup today, right? [15:51:51] I tried to hard for that mannerism. [15:53:03] * MatmaRex googles the idiom [15:53:15] Basically, are we having the standup [15:53:19] Because we are all here [15:53:25] :D [15:53:30] * marktraceur should never try to be folksy, it's not his bag [15:53:37] i'm around, can chat [15:53:38] * prtksxna looks for a ball [15:53:44] prtksxna hush [15:53:56] :P [15:54:11] Are we having an IRC meeting instead? [15:58:25] No, it's not time yet [15:58:33] Sorry I was just asking if we were cancelling [15:59:48] *now* it's time. [16:01:27] aaa [16:02:04] +marktraceur: I'm still confused. I can't seem to find the gender. Is it here? https://dpaste.de/sdAV [16:07:12] MtDu: The line you linked to, the one that called setImageInfo, has a variable in it called userInfo. That object *has the gender variable that you need*. [16:07:26] MtDu: It's not used anywhere currently, so looking for the word "gender" in the code won't help you. [16:07:58] +marktraceur: When I look around for userInfo, I don't find it. How do you know it has Gender in it? [16:08:21] Because the API returns the gender for the user, that's the point [16:08:28] MtDu: https://dpaste.de/OerL#L363 [16:08:44] 6Multimedia, 10UploadWizard, 7Performance: UploadWizard UI performance abysmal when adding many files to upload at once - https://phabricator.wikimedia.org/T121901#1894937 (10matmarex) [16:09:34] +marktraceur: So how do I get the gender and pass it to the function I need it in? [16:09:54] Well, that's the trick [16:10:40] MtDu: Looking [16:13:53] MtDu: OK, it took a lot of grepping and following of code, but I've got it. [16:14:04] MtDu: in mmv.js, look for "setFileReuseData" [16:14:24] +marktraceur: Got it. [16:15:04] grep -rn is your friend [16:15:11] MtDu: That function calls this.downloadDialog.set, which calls this.download.set, which calls this.setAttributionText, which calls this.formatter.getCreditText. [16:15:28] MtDu: You have to pass userInfo, or at least the gender, along that entire chain. [16:16:00] Hydronium: -rin is my jam [16:16:15] +marktraceur: Like this? https://dpaste.de/squx [16:16:19] Mostly because UploadWizard is stupid when it comes to naming conventions [16:16:28] MtDu: Roughly yes. [16:16:31] ooh, case insensitive [16:16:33] fancy [16:17:12] +marktraceur: What do you mean? Is it still wrong? [16:17:26] +marktraceur: And how did you know what the function claled? [16:17:26] Wait a minute. [16:17:34] Now I'm doubting the whole gender thing [16:18:03] +marktraceur: yeah. It still fails the Qunit test. [16:18:11] Oh, qunit is useless [16:20:04] +marktraceur: Ok. [16:20:16] OK, no, I was wrong about which API module was being used [16:20:18] +marktraceur: So the adding a userInfo.gender is not right. [16:20:24] +marktraceur: Ok [16:20:28] meta=userinfo doesn't give me gender, but list=users does [16:20:48] And yes, userInfo.gender is right [16:20:51] So pass that along [16:21:07] +marktraceur: Ok. [16:21:29] +marktraceur: What else do I need to do? [16:21:57] Well, pass that gender into the message as the last parameter like we planned to do with the username [16:22:23] +marktraceur: like this? https://dpaste.de/mSH4 [16:22:41] That is how you add an argument to a function, yes. [16:23:49] +marktraceur: Ok. So now, how do I pass that to getCreditHtml? [16:24:30] MtDu: My message above where I said "That function calls X, which calls Y, which calls Z"? Follow every single one of those calls and pass the gender through to them. [16:24:52] MtDu: Eventually you come to one that calls getCreditText and getCreditHtml, so then you have to pass the gender in to both. [16:25:00] MtDu: Then you can work on the functions themselves. [16:25:05] +marktraceur: Ok. How did you know that was the order? [16:25:41] Because I actually read through the code [16:26:36] +marktraceur: Derp. I'm stupid. it was right there the whole time. [16:26:52] You're not stupid, it's a complicated project [16:27:39] +marktraceur: So do I need to pass the gender into fileReuse.set as well? https://dpaste.de/D372 [16:28:19] Well, that's arguably a good idea [16:28:28] I guess the bug says "and other messages" [16:28:34] So yeah, might as well get the fileReuse ones too [16:30:08] +marktraceur: Ok. [16:30:43] +marktraceur: So I need to pass parameter every time the function this.download.set is called? [16:31:35] MtDu: All two times, yes. [16:31:45] MtDu: That means saving the gender in setValues [16:33:44] +marktraceur: Is this right? https://dpaste.de/FHTs#L60,65,84 [16:35:06] MtDu: What's the value of userInfo in DP.openDialog? [16:35:36] UND [16:35:49] So why are you trying to access userInfo.gender? [16:36:02] So can I pass in userInfo into the function? [16:36:13] No, because it gets called as a callback. [16:36:33] MtDu: Where do the other two arguments come from? [16:37:25] +marktraceur: this.setValues, so it should be this.setValues.gender [16:37:30] Yup! [16:37:58] Does that mean the upper portion is wrong? https://dpaste.de/ZOOX#L1,3,8 [16:38:18] Partly, but not the part you think [16:38:28] MtDu: What's the value of userInfo in DP.set? [16:38:41] UND. [16:38:45] Why? [16:38:54] It's not passed in. [16:38:59] So do I just pass userInfo in? [16:39:01] Isn't it? [16:39:11] I thought we just added userInfo to the only call to this function. [16:40:30] We did it here? this.downloadDialog.set( image, repo, userInfo.gender ); [16:40:54] Uhhh [16:41:15] MtDu: I fear you might be doing the same thing in every function. Are you passing userInfo.gender into each one? [16:41:33] Yeah. So far. [16:41:35] Because if you collapsed that chain of operations it would look like you were accessing userInfo.gender.gender.gender.gender.gender [16:41:58] And as interesting as it might be to know the user's meta-meta-meta-meta-gender, I don't think we're allowed to collect that information. [16:42:25] Let's just pass in userInfo.gender and then call it "gender" instead of "userInfo" from then on. [16:43:33] +marktraceur: Ok. So I start with this.downloadDialog.set [16:43:54] And pass in userInfo.gender [16:44:11] Into that and the fileReuse.set [16:44:29] Well, you start in mmv.js at the call to viewer.ui.setFileReuseData [16:44:53] yeah. I did that. [16:45:01] userInfo.gender [16:45:14] Then I did it to the two functions above. [16:45:31] But that's the cause of the userInfo.gender.gender.gender.gender problem. [16:45:45] So only use userInfo in mmv.js, the rest of the time, it's just gender. [16:45:57] +marktraceur: Gotcha. [16:46:28] So now I'm on this.download.set. [16:46:39] Is this right? https://dpaste.de/Y4fT [16:46:50] Or do I need to change line 8 to just gender? [16:47:30] MtDu: It should only be gender, userInfo is not present outside of mmv.js [16:47:37] Ok. [16:47:43]