[00:18:24] * marktraceur facepalms [00:18:31] There was a bug in the EL code [00:18:43] We have zero load times for, well, everything [00:18:54] Which would be really impressive if it wasn't just a stupid bug [00:19:01] I blame testing [00:22:35] (03PS1) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 [00:23:03] (03CR) 10jenkins-bot: [V: 04-1] Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 (owner: 10MarkTraceur) [00:24:54] (03PS2) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 [00:25:00] Cleverer than I give myself credit for [00:27:04] tgr: So ^^ that will fix our metrics [00:27:06] Too late, but still [00:27:12] I'll write that email [00:34:53] I guess I could try to backport it maybe, I don't think image bucketing will hit e.g. enwiki 'til Thursday [00:35:00] Wonder if greg-g would go for it [00:36:16] Probably yes, it's not a complicated thing [00:37:10] worst case, it breaks logging even more, right? [00:37:20] Well, let's bolody hope not [00:37:23] bloody* [00:37:23] nothing users would be affected by [00:37:56] Correct [00:38:22] I mean, worst case it contains a fatal error we don't catch, but the tests are there for a reason [00:38:58] Also I've confirmed it works locally [01:04:56] (03PS3) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 [01:05:02] Now with tests [01:05:24] (03CR) 10jenkins-bot: [V: 04-1] Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 (owner: 10MarkTraceur) [01:05:29] Failing tests woo [01:06:10] (03PS4) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 [01:06:38] (03CR) 10jenkins-bot: [V: 04-1] Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 (owner: 10MarkTraceur) [01:06:42] (03PS5) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 [01:06:56] Hrm. [01:07:11] (03CR) 10jenkins-bot: [V: 04-1] Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 (owner: 10MarkTraceur) [01:07:29] That's really strange [01:07:38] I'll tear this out and put it in a different patchset [01:07:54] (03PS6) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 [01:08:29] (03PS1) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [01:08:57] (03CR) 10jenkins-bot: [V: 04-1] Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 (owner: 10MarkTraceur) [01:12:22] (03PS2) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [01:14:10] Seriously Jenkins? [01:14:13] Because if I try again [01:14:17] You're totally going to fail it [01:14:30] (03PS3) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [01:14:57] (03CR) 10jenkins-bot: [V: 04-1] Add tests for EventLogging timing. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 (owner: 10MarkTraceur) [01:17:11] (03PS4) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [01:17:33] I will be super angry if this doesn't pass consistently [01:19:03] (03PS5) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [01:19:52] I am suuuuuper skeptical [01:20:25] (03PS6) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [01:23:38] (03PS7) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [01:23:52] Dumb bug #2 I guess [01:24:45] OK, that one seems better. [01:26:10] (03PS7) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 [01:26:37] (03CR) 10jenkins-bot: [V: 04-1] Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 (owner: 10MarkTraceur) [01:26:44] (03PS8) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [01:27:11] (03CR) 10jenkins-bot: [V: 04-1] Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 (owner: 10MarkTraceur) [01:27:33] (03PS8) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 [01:27:41] (03PS9) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [01:27:54] * marktraceur breathes [01:35:16] tgr: Sorry that patch got dumped on, I wanted to get the tests working and then needed to disentangle the tests, they should both be good now [01:48:03] I guess it doesn't matter, we can't deploy 'til tomorrow anyway [01:48:29] (03CR) 10Gergő Tisza: [C: 031] Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 (owner: 10MarkTraceur) [01:49:26] ? [01:55:35] ? what? [01:55:39] Only 1? [01:56:05] is it urgent? [01:56:12] I mean...not today-urgent [01:56:16] Tomorrow-urgent though [01:58:06] BRB, picking up dinner [01:58:32] i figured we should leave changesets open at least for a full night so aarcos and gi11es-away can comment [01:59:10] i have nothing against the patch, i can +2 if you want it sooner than that [02:04:05] 'kay [02:04:09] Tonight is fine [02:04:26] As long as it can get pushed out tomorrow [02:20:12] (03CR) 10Gergő Tisza: Add tests for EventLogging timing (031 comment) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 (owner: 10MarkTraceur) [02:24:29] (03PS10) 10MarkTraceur: Add tests for EventLogging timing [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 [02:24:30] {{done}} [02:53:39] (03CR) 10Gergő Tisza: [C: 031] "Looks good. I would still prefer if the non-assertion-related logic (like starting the next profiling session) would be outside logEvent()" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106176 (owner: 10MarkTraceur) [10:34:15] (03PS1) 10Gilles: Move the arrows and the close/fullscreen button [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106224 [10:52:49] (03PS2) 10Gilles: Solves visibility issue on the progress bar [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105694 [13:42:58] (03PS1) 10Pginer: Use chevron for panel opening affordance [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106244 [13:43:00] (03CR) 10Theopolisme: [C: 04-1] "Thanks again! This is looking good." [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105459 (owner: 10Apsdehal) [13:47:49] (03PS2) 10Theopolisme: Changed the font-size for decription in lightbox [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105154 (owner: 10Apsdehal) [13:47:50] (03CR) 10jenkins-bot: [V: 04-1] Reduce font-size for description in lightbox [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105154 (owner: 10Apsdehal) [13:48:21] (03CR) 10Theopolisme: [C: 031] "Looks good, needs rebase though" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105154 (owner: 10Apsdehal) [14:30:25] (03PS3) 10Apsdehal: Reduce font-size for description in lightbox [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105154 [14:42:53] (03PS8) 10Apsdehal: Titles attribute added to fullscreen and close button [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105459 [15:15:35] (03PS1) 10Gilles: Make the behavior of the lightbox more consistent with scroll [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106263 [15:17:26] (03CR) 10jenkins-bot: [V: 04-1] Make the behavior of the lightbox more consistent with scroll [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106263 (owner: 10Gilles) [15:39:24] aarcos, gi11es, tgr and I left https://gerrit.wikimedia.org/r/106166 open primarily so you could take a look, but I'd like to get it merged today if you have time :) [15:52:34] Agh, pginer isn't in [15:52:40] I'm so happy with how he made the SVG [15:54:34] Um, did we decide that SSH was totally not working for Gerrit? [15:58:14] it's still required, afaik? without adding the ssh key to gerrit you can't connect [15:58:26] at least that was my experience messing with it this week [15:58:38] Well, I have my key added [15:58:55] pginer: <3 I love the new drag affordance, especially the way it's built in the SVG file [15:58:56] I had to redo mine today, not sure why [15:59:12] Hm, I'm getting "no route to host" [15:59:14] Which is crap [15:59:25] marktraceur: thanks [16:03:11] (03CR) 10MarkTraceur: [C: 04-2] "Hi!" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/104325 (owner: 10Apsdehal) [16:14:48] (03PS2) 10Gilles: Make the behavior of the lightbox more consistent with scroll [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106263 [16:16:32] (03CR) 10MarkTraceur: [C: 032] "Huzzah, arrows!" (031 comment) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106244 (owner: 10Pginer) [16:16:44] OK [16:16:51] I should stop WFHing and go to office [16:16:59] (03Merged) 10jenkins-bot: Use chevron for panel opening affordance [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106244 (owner: 10Pginer) [17:14:29] does anyone know on the top of their head where in the code chunked upload are being reassembled/processed? [17:14:43] Lol. [17:14:57] gi11es: includes/jobs/AssembleChunkedUploads.php off the top of my head [17:15:04] thanks [17:15:25] Sorry [17:15:34] includes/job/jobs/AssembleChunkedUploads.php [17:15:41] Nope still wrong [17:15:51] (03PS4) 10M4tx: Add support for more Flickr URLs [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105393 [17:15:51] includes/job/jobs/AssembleUploadChunksJob.php [17:15:53] (03CR) 10jenkins-bot: [V: 04-1] Add support for more Flickr URLs [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105393 (owner: 10M4tx) [17:16:25] The job queue is a mysterious beast [17:17:13] (03CR) 10Aarcos: [C: 032] Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 (owner: 10MarkTraceur) [17:18:03] \o/ [17:18:07] is the job queue purely php? [17:18:23] Yeah, I believe so [17:18:46] (03Merged) 10jenkins-bot: Fix painful bug in eventlogging code [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106166 (owner: 10MarkTraceur) [17:21:26] Oh, crap, if I were smart I would have put a different version number in [17:21:30] D'oh [17:21:49] Hopefully I can get it in today [17:23:47] (03PS1) 10MarkTraceur: Version our EL profiling [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106282 [17:23:54] aarcos: Quick add-on ^^ [17:28:04] (03CR) 10Aarcos: [C: 032] Version our EL profiling [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106282 (owner: 10MarkTraceur) [17:31:14] THankee! [17:32:39] All right, we should be set to generate stats [17:32:43] Once I backport [17:33:02] We'll only get about...eh...a night of data from non-bucketing code? I think? [17:34:53] (03CR) 10jenkins-bot: [V: 04-1] Version our EL profiling [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106282 (owner: 10MarkTraceur) [17:34:58] ...wat [17:35:18] Oh, of course [17:35:24] (03CR) 10jenkins-bot: [V: 04-1] Version our EL profiling [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106282 (owner: 10MarkTraceur) [17:35:40] (03PS2) 10MarkTraceur: Version our EL profiling [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106282 [17:35:42] I'm terrible [17:36:21] aarcos: Because testing ^^ [17:37:18] lol [17:37:42] (03CR) 10Aarcos: [C: 032] Version our EL profiling [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106282 (owner: 10MarkTraceur) [17:38:11] (03Merged) 10jenkins-bot: Version our EL profiling [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106282 (owner: 10MarkTraceur) [17:39:14] 'kay, will make branches for us and everything [17:48:58] (03PS1) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] (wmf/1.23wmf8) - 10https://gerrit.wikimedia.org/r/106287 [17:49:33] (03CR) 10MarkTraceur: [C: 032] "Sometimes Gerrit just deserves a lot of love." [extensions/MultimediaViewer] (wmf/1.23wmf8) - 10https://gerrit.wikimedia.org/r/106287 (owner: 10MarkTraceur) [17:49:48] (03PS1) 10MarkTraceur: Fix painful bug in eventlogging code [extensions/MultimediaViewer] (wmf/1.23wmf9) - 10https://gerrit.wikimedia.org/r/106288 [17:50:01] (03Merged) 10jenkins-bot: Fix painful bug in eventlogging code [extensions/MultimediaViewer] (wmf/1.23wmf8) - 10https://gerrit.wikimedia.org/r/106287 (owner: 10MarkTraceur) [17:50:12] (03CR) 10MarkTraceur: [C: 032] "Muahahah." [extensions/MultimediaViewer] (wmf/1.23wmf9) - 10https://gerrit.wikimedia.org/r/106288 (owner: 10MarkTraceur) [17:50:27] (03PS1) 10MarkTraceur: Version our EL profiling [extensions/MultimediaViewer] (wmf/1.23wmf8) - 10https://gerrit.wikimedia.org/r/106290 [17:50:41] (03PS1) 10M4tx: Merge branch 'master' of ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard into bug/42964 [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/106291 [17:50:42] (03CR) 10MarkTraceur: [C: 032] Version our EL profiling [extensions/MultimediaViewer] (wmf/1.23wmf8) - 10https://gerrit.wikimedia.org/r/106290 (owner: 10MarkTraceur) [17:50:51] (03PS1) 10MarkTraceur: Version our EL profiling [extensions/MultimediaViewer] (wmf/1.23wmf9) - 10https://gerrit.wikimedia.org/r/106292 [17:50:58] (03Merged) 10jenkins-bot: Fix painful bug in eventlogging code [extensions/MultimediaViewer] (wmf/1.23wmf9) - 10https://gerrit.wikimedia.org/r/106288 (owner: 10MarkTraceur) [17:51:13] (03CR) 10MarkTraceur: [C: 032] Version our EL profiling [extensions/MultimediaViewer] (wmf/1.23wmf9) - 10https://gerrit.wikimedia.org/r/106292 (owner: 10MarkTraceur) [17:51:24] Patchspam yayyyy [17:51:49] (03Abandoned) 10M4tx: Merge branch 'master' of ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard into bug/42964 [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/106291 (owner: 10M4tx) [17:52:01] (03Merged) 10jenkins-bot: Version our EL profiling [extensions/MultimediaViewer] (wmf/1.23wmf8) - 10https://gerrit.wikimedia.org/r/106290 (owner: 10MarkTraceur) [17:52:38] Hm, I don't have a tgr yet, and the standup is soon [17:52:54] (03Merged) 10jenkins-bot: Version our EL profiling [extensions/MultimediaViewer] (wmf/1.23wmf9) - 10https://gerrit.wikimedia.org/r/106292 (owner: 10MarkTraceur) [17:52:57] Oh wait, even worse, the bug triage is soon [17:55:28] (03PS5) 10M4tx: Add support for more Flickr URLs [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105393 [17:55:30] (03CR) 10jenkins-bot: [V: 04-1] Add support for more Flickr URLs [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105393 (owner: 10M4tx) [17:58:44] both entries in the calendar overlap, which video call should we join first? daily scrum? [17:59:55] I think triage [18:00:50] We have some A/V issues, predictably [18:01:12] (03PS6) 10M4tx: Add support for more Flickr URLs [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105393 [18:04:02] Hi marktraceur and tgr : Are you joining us on Google Hangouts? [18:04:11] trying to [18:04:23] we have some tech problems [18:04:39] ok, we are waiting... [18:05:17] (03CR) 10M4tx: "Sorry for the mess. I had some problems with merging the patch. I fixed the problems that Gergő Tisza mentioned." [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105393 (owner: 10M4tx) [18:14:02] Which room are you guys in? Diderot? [18:21:08] (03CR) 10Siebrand: [C: 031] "i18n/L10n reviewed." [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105393 (owner: 10M4tx) [18:51:01] as a side topic, do we do any client-side logging of JS errors? could be useful with bugs like this [19:03:28] tgr: Hm, I think there is some logging, but I'm forgetting how it works [19:11:26] fabriceflorin: OK, re: spinner [19:11:39] I think the situation is, with the progressive load patch, it's not shown anymore [19:11:42] But that patch needs fixing [19:11:50] So that should maybe be priority one for me [19:14:38] marktraceur: yes, it would be great if we could fix the spinner today for tomorrow's release, it makes us all look bad. Even if you can't implement progressive image load today, at least replace the spinner to use Pau's instead -- or a better version of yours that looks good. We're staring at this for up to 10 seconds, so it's perceived as our primary experience, hence the high priority. [19:15:05] Wait. Pau's spinner? [19:15:11] I don't remember that existing [19:15:46] Pau had recommended a spinner in an email he sent in December. Let's ask him now and/or dig up through email. [19:16:05] I'll look [19:16:51] this is just about fixing the problem in https://gerrit.wikimedia.org/r/#/c/100693/ right? [19:17:12] I can try if you don't mind, Mark, I'm a bit short on tasks today [19:17:25] Oh, sure [19:17:31] As long as we have a reviewer [19:17:45] I don't want to go to sleep uncertain of whether it will get merged [19:19:17] i'll take a shot at it and see if aarcos or gi11es is still up afterwards [19:19:30] 'kay [19:19:38] re logging, I was thinking of logging client errors on server side [19:19:45] Oh, I see [19:19:50] catch exceptions at top level, send home in AJAX [19:20:06] do we have something like that? [19:20:12] There might be a way in the debug settings, but I don't know [19:20:14] ori-l might [19:28:40] Hey gi11es, I have some patches that are mostly refactoring that you could probably review quick: [19:28:43] :* https://gerrit.wikimedia.org/r/105993 [19:28:45] :* https://gerrit.wikimedia.org/r/106004 [19:28:48] :* https://gerrit.wikimedia.org/r/106005 [19:29:14] fabriceflorin: FYI there are issues with theopolisme's core patch for fullscreen that may stop it from going out [19:29:27] But the next version can easily have it [19:30:13] marktraceur: Full screen can wait, but let's fix the spinner and the metadata panel that obscures the image for tomorrow's release. [19:30:25] OK [19:30:35] I'll review the metadata panel thing now [19:30:45] And back off intensity on the fullscreen things [19:32:52] * Test image bucketing code (in parallel with metrics) (who?) [19:33:00] fabriceflorin: Who was supposed to be doing this? [19:35:50] (03CR) 10MarkTraceur: [C: 032] "I thought it might not work with document instead of document.body, but WFM. Thanks, gi11es!" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106263 (owner: 10Gilles) [19:36:18] (03Merged) 10jenkins-bot: Make the behavior of the lightbox more consistent with scroll [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/106263 (owner: 10Gilles) [19:36:33] {{done}}, wonderful [19:38:37] aarcos, i dont understand your comments on https://gerrit.wikimedia.org/r/103596 , i have fixed the code duplication by defining a new function [19:42:02] mayankmadan: Lines 69-78 look a lot like lines 80-85. [19:42:53] And 98-105 [19:43:17] And 114-123 [19:44:06] marktraceur, but an apiRequest has to made for every test [19:44:12] Yes [19:44:29] And you can make that API request in a different function [19:44:52] I'm guessing about aarcos's intent, but I suspect they involved repeated code there [19:45:00] Also grumble, where did fabriceflorin get to [19:46:22] marktraceur, okay [20:06:38] (03CR) 10Kaldari: "@M4tx: I wasn't complaining. It was a compliment!" [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105393 (owner: 10M4tx) [20:08:48] (03CR) 10Kaldari: [C: 031] Add support for more Flickr URLs [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105393 (owner: 10M4tx) [20:18:35] marktraceur: Sorry, I was away, due to power outage. Will be back on IRC for the rest of the day (though breaking for lunch shortly). Glad you're focusing on spinner and metadata panel obstruction for tomorrow's release, they are both key for us, full screen can wait. [20:18:46] 'kay [20:19:05] fabriceflorin: What about the question about testing image bucketing? [20:20:36] marktraceur: I thought we discussed asking Aaron to work with you to test image bucketing, analyze results and report on findings. Perhaps we could start an email thread or ether pad for now to see what we can realistically do with limited resources to see how painful this is for users and how much our bucketing code is helping with that. [20:20:50] 'kay [20:20:57] I just didn't have it written down in the pad [20:21:27] I can actually start analysing the data this week, I think [20:21:41] Like, on a dashboard [20:21:44] Cool. Thanks for all your good work this week, and sorry I haven't been more responsive, due to a variety of factors. [20:21:53] No problem [20:22:56] Moving on now to writing the monthly engineering report, planning community engagement with Keegan and posting the vision piece later today. Ping me if you need anything tested for tomorrow's Media Viewer release. Cheers ... [20:23:31] Sure sure [20:25:01] marktraceur: is it ok if I review those 10 hours from now? I really need a bit of free time tonight. reviews are the first thing I do when I get up [20:25:43] I'll do [20:25:50] (03PS2) 10Gergő Tisza: Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [20:25:50] (03CR) 10jenkins-bot: [V: 04-1] Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [20:26:16] (03CR) 10Gergő Tisza: "Just adding some jsdoc." [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [20:54:28] (03PS11) 10Mayankmadan: Functionality to create a new image in upload-wizard_tests.py [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/103596 [20:54:31] (03CR) 10jenkins-bot: [V: 04-1] Functionality to create a new image in upload-wizard_tests.py [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/103596 (owner: 10Mayankmadan) [20:55:22] marktraceur, how is the latest patchset looking? [20:56:14] Basically the same? [20:56:43] Ah, you're almost there [20:56:48] Just call that function a few more times [20:57:07] tgr: Are you signing up to do GSoC this year? I think I'm a bit disillusioned about it [20:57:16] (there's a post on the list) [20:58:12] will see how OPW goes [20:58:26] so far i havent done a great job, i guess [20:58:40] marktraceur, was the "Just call that function a few more times" for me? [20:59:57] (03PS3) 10Gergő Tisza: Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [21:00:07] Yeah [21:00:13] mayankmadan: Yes, it was [21:00:25] (03CR) 10jenkins-bot: [V: 04-1] Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [21:00:30] marktraceur, a few more times meaning in other tests too? [21:00:45] Ideally, or your refactor might not make much sense [21:00:49] (03CR) 10Gergő Tisza: "Rebased. I think the problem is related to race conditions in the API calls, and the recent work with promises makes it a lot easier to de" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [21:02:02] aw crap, those are not merged yet [21:02:11] Nooope [21:02:15] * marktraceur looks at gi11es  [21:03:47] http://24.media.tumblr.com/tumblr_mco3mh94gd1qgc2meo1_400.jpg [21:04:31] I promise that you won't have to wait more than 24 hours for reviews from me in the future, it's just that start week comes with a lot of extra overhead [21:04:42] and I really need to research strollers tonight [21:05:09] Heh [21:05:10] hm, jshint does not like some of the closure documentation style [21:05:47] tgr: I think just basic jsduck is probably better :/ [21:06:07] the issue is with [21:06:16] /** .... */ [21:06:28] Foo.bar; [21:06:31] Really? [21:06:46] which is not an assignment or a function call [21:06:57] ...huh [21:07:01] but a nice way to document members in a prototype [21:07:21] could just doo Foo.bar = undefined; i guess [21:07:58] Wait. [21:08:01] What? [21:08:22] Oh [21:08:35] i added a block like [21:08:37] /** [21:08:37] * @type {LightboxImage} [21:08:37] * @protected [21:08:37] */ [21:08:39] LIP.currentImage; [21:08:52] just to get type annotation on currentImage [21:08:54] You can do that in the constructor [21:09:58] yes, but when someone wants to read through what the constructor actually does, he wouldn't thank me [21:11:06] True. [21:11:13] OK, so undefined or null would also work [21:11:27] yeah, I'll do that [21:12:11] (03PS4) 10Gergő Tisza: Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [21:16:34] the discussion happened before I was here, but yes, spinners for images are bad [21:16:55] we studied the effect at deviantart and real image load beats speed impression of spinner hands down [21:17:20] even better if you can blow up any lower res image already cached and load the higher res on top [21:21:10] and while it's generally better to have an entire UI appear at once when everything is ready, instead of people seeing the DOM move around as it loads/gets constructed, images are a notable exception in that it's better if people see them appear [21:21:36] basically you don't want stuff to jump around, but it's better to have images load the naturel top-to-bottom way [21:21:39] (03PS12) 10Mayankmadan: Functionality to create a new image in upload-wizard_tests.py [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/103596 [21:22:22] marktraceur, now it should be good to go [21:22:35] I've run into situations where the choice about what to wait for/what to display right away had really counter-intuitive results in what felt faster [21:34:10] 2014-01-08 - 13:17:20 even better if you can blow up any lower res image already cached and load the higher res on top [21:34:17] gi11es: What do you mean exactly, there? [21:35:02] if the thumb on the page is big enough, you can but it as the lightbox's background right behind the loading image [21:35:22] it only works if the thumb is reasonably big to begin with, but we did that a bunch on dA [21:35:35] *put it [21:37:19] Well, the thumbnail would be of varying size [21:37:32] There can be thumbnails that are 2 pixels and thumbnails that are 2000 pixels [21:37:37] We don't discriminate [21:37:52] we could set a maximum blowup factor [21:38:05] Hm, maybe [21:38:13] i.e. only do this if you try to display the thumb at most 6 times its original size or something like that [21:38:16] But then it's inconsistent and seeing no image may be shocking [21:38:23] true [21:38:43] I think it's worth experimenting with, in practice most thumbs are at least 140px, right? [21:38:50] Usually, yes [21:39:31] I honestly can't remember the size of the ones we were using on dA, 140px might look utterly crappy, but I'd like to see it to be sure [21:39:41] Heh [21:39:52] We can work on that after the image resize thing goes in maybe [21:40:03] Right now I think I'll focus on getting the analytics scripts to run every day [21:50:54] (03CR) 10Theopolisme: [C: 04-1] Titles attribute added to fullscreen and close button (031 comment) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105459 (owner: 10Apsdehal) [21:52:03] (03CR) 10Theopolisme: Reduce font-size for description in lightbox (031 comment) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105154 (owner: 10Apsdehal) [21:57:06] Maybe even some sort of useful dashboard! [21:57:08] Exciting times [21:57:14] I have two hours before the LD [21:58:13] marktraceur, gi11es default thumb size is 220px, since late 2009. bug: https://bugzilla.wikimedia.org/show_bug.cgi?id=21117 Enwiki thread: https://en.wikipedia.org/wiki/Wikipedia_talk:Image_use_policy/Archive_12#Proposal_to_increase_the_default_thumbnail_dimensions [21:58:28] neat, thanks for the info [22:44:30] (03PS5) 10Gergő Tisza: Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [22:45:31] (03CR) 10jenkins-bot: [V: 04-1] Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [22:56:06] (03CR) 10Gergő Tisza: "Fixed some places in the code where data was not passed correctly. This fixed a lot of resize bugs, but the image loading is not increment" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [22:59:28] marktraceur: Do you think anything tested before the LD? [22:59:55] The LD shouldn't be anything complicated, I have the patches set up in the wmf8/9 branches and it should be quick and uneventful [23:04:31] Okay, sounds good. Were you able to fix the spinner and the media panel obstruction? [23:09:50] (03CR) 10Gergő Tisza: "Incremental display still does not work even if I undo all my changes. Must have been caused by the rebase." [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [23:13:56] fabriceflorin: tgr is working on it [23:15:02] super. thanks for the update. [23:16:36] tgr: The incremental loading may not work obviously, because localhost may be too fast to notice it [23:16:46] You should probably go for a *really* big image to test. [23:17:26] i'm thinking of that too, since it seems to work every once in a while [23:17:43] is it possible that the image is not displayed before the metadata is in? [23:18:05] It's definitely not. [23:18:14] because it works very sluggishly for me on localhost, which makes sense for PHP but not for images [23:18:15] Because the metadata fetch also includes the URL to the thumbnail [23:18:38] ah, yes, of course [23:19:06] still, it feels like something is blocking display of the image when it is loading already [23:19:54] Hm. There *shouldn't* be, but I dunno [23:20:12] nevermind [23:20:32] i played around with cache settings in chrome, and now incremental loading works fine [23:20:44] Woot [23:20:55] so i guess the patch is ready for review [23:21:06] 'kay [23:21:14] I'm the only one left, gah [23:21:19] I'll review it anyway [23:21:31] If you can +1 that the bits I wrote make sense [23:23:20] i think i fixed some bugs that you already fixed in the resize engine patches, so there might be a conflict when those are merged [23:23:31] Heh, fun [23:23:37] I'll deal with that after tomorrow anyway [23:23:53] I thought at some point that a resize callback is missed, but it was a different issue after all [23:24:37] Did you review the parent commits? [23:24:52] parent of what? [23:24:53] It looks like there's an older version actually [23:25:04] Oh, never mind [23:26:07] (03PS6) 10Gergő Tisza: Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [23:26:18] I forgot a console.log there [23:26:37] (03CR) 10jenkins-bot: [V: 04-1] Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [23:27:01] tgr: Won't the image.width = targetWidth need to go before the image.src = line now? [23:27:16] Or immediately after [23:28:43] not sure when an image gets a width [23:28:59] I recall looking for an event to listen for and failing. [23:29:13] Maybe just set a timeout that looks for the width? [23:29:28] If it's undefined set another timeout etc. [23:29:58] probably you are right and immediately after would work [23:30:02] lets see [23:30:27] The trick is, before I wasn't looking at the width to see if it was bigger. [23:30:33] So now that we have that...hm. [23:31:49] tgr: Aha - check the "thumbwidth" value from the API response, it will tell you the size of the URL [23:31:56] s/URL/image/ wow [23:32:09] If that's bigger, then replace the width before even loading the image. [23:33:55] Should be innerInfo.thumbwidth [23:36:22] seems to work [23:36:31] or at least there is no visible difference [23:37:04] (03PS7) 10Gergő Tisza: Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [23:37:33] (03CR) 10jenkins-bot: [V: 04-1] Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [23:43:38] (03PS8) 10Gergő Tisza: Load images normally [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100693 (owner: 10MarkTraceur) [23:43:52] got rid of some assertions, I don't think they make sense anymore [23:43:57] 'kay [23:44:12] and hopefully this makes jenkins happy [23:44:47] We can only hope [23:45:44] ok, +1 on my side [23:53:53] tgr: How do you make Chrome not be dumb? [23:54:15] you mean caching? [23:54:19] Yeah [23:54:24] I want to verify it's working [23:54:40] i think opening the web dev toolbar and enabling the no cache option helped [23:54:56] you have to keep the toolbar open for it to work [23:55:15] Hm [23:55:25] I think I may need to also have a big file locally [23:57:07] Still not seeing it, weird. [23:58:05] I'll do the deploy and try again after