[16:28:30] (03PS9) 10Aarcos: Add a tests to MultimediaViewer. Based on https://gerrit.wikimedia.org/r/#/c/96152/1 These are just smoke tests. I will add more in coming versions of this change. Consolidated various cases in one tests. Added tests to validate for legit clicks. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 [16:34:28] aarcos: I guess I should review your stuff [16:35:14] marktraceur: that would be nice, ;-). [16:35:29] First thing when I get in, sah! [16:36:00] marktraceur: tx ! [16:51:12] (03CR) 10Qgil: "Got a bit lost here. Can this GCI task be closed as completed, or are we still waiting for a fix from vldandrew?" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98180 (owner: 10Vldandrew) [16:55:43] (03CR) 10Vldandrew: "The first problem was resolved, now the problem is if the link is to a remote server, we will wind up with something like https://en.wikip" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98180 (owner: 10Vldandrew) [17:26:15] (03PS1) 10Gergő Tisza: Add tests for InformationParser [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/98546 [17:28:11] (03PS2) 10Gergő Tisza: WIP Add tests for InformationParser [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/98546 [18:26:04] (03PS10) 10MarkTraceur: Add a tests to MultimediaViewer. Based on https://gerrit.wikimedia.org/r/#/c/96152/1 These are just smoke tests. I will add more in coming versions of this change. Consolidated various cases in one tests. Added tests to validate for legit clicks. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [18:26:23] (03CR) 10MarkTraceur: "Changed commit message again, please pull before updating again :)" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [18:39:57] (03PS5) 10Aarcos: Load image only if data was returned, otherwise do nothing. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/96682 [18:41:36] (03PS3) 10MarkTraceur: Bump version number [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/96411 [18:41:53] (03CR) 10MarkTraceur: [C: 032] "Sue me, this is trivial." [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/96411 (owner: 10MarkTraceur) [18:41:56] (03Merged) 10jenkins-bot: Bump version number [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/96411 (owner: 10MarkTraceur) [18:42:05] Needed to happen [19:12:41] (03CR) 10MarkTraceur: [C: 04-1] "So, I'm being OCD, but there are loads of spacing and jshint issues in the new code." (0329 comments) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [19:13:07] aarcos: spacing, jshint, and jquery use could be better, but otherwise it's looking good - I'll test it async. [19:15:16] I'll get jshint running on patches for us so we can be held accountable [19:16:30] marktraceur: great !, I will quickly fix the formatting issues. [19:17:10] 'kay [19:32:01] aarcos: https://gerrit.wikimedia.org/r/#/c/98583/ [19:32:09] so jenkins runs tests [19:38:07] aude: lgtm but someone else has to approve, ;-). [19:38:20] OOH OOH PICK ME [19:38:58] aarcos: hashar usually handles it [19:39:17] * aude just notices new people not whitelisted yet [19:40:02] {{done}} [19:40:17] I guess I should deploy that [19:40:23] cool :) [19:40:25] But I will do that after I also enable jshint voting [19:40:30] Like a boss, etc. [19:44:01] (03PS1) 10MarkTraceur: jshint config and fixes, FINALLY [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98591 [19:44:11] aarcos or tgr, you'd be my favourite if you reviewed that [19:44:20] I'm pretty fickle, but still [19:47:27] (03PS1) 10MarkTraceur: Fix fileLink initialisation [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98593 [19:47:40] Less critical but annoying [19:54:40] aude: Are you +2'd on zuul-config? [19:56:16] not yet [19:56:43] anyway makes sense for someone who can deploy [20:13:33] marktraceur: i don't see any 'regexp' option in the jshint docs [20:13:55] other than that, seems good to me [20:15:53] i'm not a fan of banning weak equals, but given that i have broken my first patch at the WMF with one, i probably should be silent about that [20:23:07] (03CR) 10Gergő Tisza: "Should this actually cause the thumbnail image the appear as a placeholder while the large image is loading? I don't see that happening." [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98593 (owner: 10MarkTraceur) [20:30:40] (03CR) 10Gergő Tisza: jshint config and fixes, FINALLY (034 comments) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98591 (owner: 10MarkTraceur) [20:31:51] tgr: Hm, I just copied the jshintrc from UploadWizard [20:32:06] Weak equals is pretty seriously hated 'round here too [20:40:07] Async, should I be enabling GWToolset on *all* beta wikis, or just betacommons? (bd808?) [20:41:06] marktraceur: Just betacommons I think. AFAIK it is only intended to end up on commons [20:41:31] 'kay [20:45:49] marktraceur: config patch looks like the right start, but I think we need config info from dan-nl [20:46:15] There are a bunch of settings that need to go along with the extension [20:48:14] Aha. [20:48:19] bd808: Good plane [20:48:22] plan* [20:48:28] * marktraceur finds more caffeine [20:49:57] (03CR) 10MarkTraceur: "Incoming" (034 comments) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98591 (owner: 10MarkTraceur) [20:50:40] (03PS2) 10MarkTraceur: jshint config and fixes, FINALLY [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98591 [20:57:37] (03CR) 10Gergő Tisza: [C: 031] jshint config and fixes, FINALLY [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98591 (owner: 10MarkTraceur) [20:59:29] Rrrrg C1 [21:11:56] (03PS1) 10MarkTraceur: Fix a bloody silly file usage dialog bug [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98690 [21:39:57] tgr or brion, https://gerrit.wikimedia.org/r/98690 is critical-ish if you have a sec, also pretty much trivial [21:40:23] I don't want to distract Aaron from unit tests, those things need to happen [21:40:33] looking [21:40:43] Thanks [21:41:07] I think after that we have one more thing I want to get out tomorrow (the affordance that pginer put in for scrollability), then I can go back to CR [21:41:25] (03CR) 10Gergő Tisza: [C: 032] Fix a bloody silly file usage dialog bug [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98690 (owner: 10MarkTraceur) [21:41:30] (03Merged) 10jenkins-bot: Fix a bloody silly file usage dialog bug [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98690 (owner: 10MarkTraceur) [21:41:51] * marktraceur plays soundtrack from "The Good, the Bad, and the Ugly" [21:42:00] Fastest merge button in the west [21:42:02] (03CR) 10Brion VIBBER: Fix a bloody silly file usage dialog bug (032 comments) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98690 (owner: 10MarkTraceur) [21:42:14] :) [21:42:35] 2 comments in a one-line patch [21:42:36] Wat [21:42:43] hah [21:42:47] well one was for existing code [21:42:51] so feel free to disregard :D [21:43:12] Also not very useful [21:43:17] Because I'm not linking to a new window [21:43:22] I'm linking to a dialog... [21:43:42] I guess I could add /usethisfile to the hash fragment and add that to the load code [21:43:49] * marktraceur sighs [21:44:03] well ideal behavior when using a link is to allow open in new window to work, otherwise you usually want a button [21:44:17] you could indeed include the info in the hash... [21:44:57] (03PS11) 10Aarcos: Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 [21:56:32] Yay, my OCD is much happier with the new patch. :P [21:56:52] aarcos: I think there are still issues with how your text editor is auto-indenting [21:56:59] (03CR) 10Aarcos: [C: 031] jshint config and fixes, FINALLY [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98591 (owner: 10MarkTraceur) [21:57:19] (03CR) 10MarkTraceur: [C: 04-1] "Minor-ish issues, just enough for -1" (033 comments) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [21:57:55] tgr: Did you fail-to-+2 for a reason? Now with both team members +1ing I'm basically OK to +2. [21:58:24] marktraceur: ok, fixing... [21:58:36] (03CR) 10MarkTraceur: Fix a bloody silly file usage dialog bug (031 comment) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98690 (owner: 10MarkTraceur) [21:58:45] *nod* should be much faster [21:59:08] marktraceur: thought aarcos might want to look at it as well, no other reason [21:59:17] Ah, cool [21:59:20] Well someone should +2 it [21:59:25] It "shouldn't" be me [22:00:01] (03CR) 10Gergő Tisza: [C: 032] jshint config and fixes, FINALLY [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98591 (owner: 10MarkTraceur) [22:00:06] (03Merged) 10jenkins-bot: jshint config and fixes, FINALLY [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98591 (owner: 10MarkTraceur) [22:00:07] Tadaaaaa [22:00:14] * marktraceur goes to merge and deploy zuul config change [22:01:04] I guess I'll be doing that again later [22:01:05] Sigh [22:08:22] (03PS4) 10MarkTraceur: File usage dialog: Only reselect on focus, not on every click [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98128 (owner: 10Bartosz Dziewoński) [22:08:47] Cool, it's voting now [22:15:08] hey bd808 thanks for those merges over the weekend [22:15:33] i didn't expect that [22:15:55] dan-nl: You're welcome. I was poking around at some other stuff and it was very easy to do. [22:19:51] (03PS12) 10Aarcos: Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 [22:20:21] (03CR) 10jenkins-bot: [V: 04-1] Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [22:20:59] Oho, and another one bites the dust [22:21:34] aarcos: https://integration.wikimedia.org/ci/job/mwext-MultimediaViewer-jslint/285/console is now going to be OCD for me. :) [22:22:30] marktraceur: fixing one more space issue in the hooks, almost there. [22:22:39] *nod* [22:25:14] (03PS13) 10Aarcos: Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 [22:25:20] (03CR) 10jenkins-bot: [V: 04-1] Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [22:26:33] I don't know why it's complaining about a missing semicolon, that seems inacrcurate [22:42:25] fixing it now... [22:43:46] (03PS14) 10Aarcos: Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 [22:43:50] (03CR) 10jenkins-bot: [V: 04-1] Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [22:52:44] wow !, now I get lots of errors, fixing them... [22:52:58] Heh [23:00:49] (03PS15) 10Aarcos: Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 [23:00:53] (03CR) 10jenkins-bot: [V: 04-1] Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [23:08:44] fixing reminding issues... [23:16:01] * marktraceur thinks [23:16:20] (03PS16) 10Aarcos: Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 [23:16:24] I'm not sure how I'm gonna pull off the static position for the image...I guess by reworking the close/next/prev buttons AGAIN. [23:16:38] (03CR) 10jenkins-bot: [V: 04-1] Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [23:16:46] You must be getting close now [23:17:22] one error left, almost, sorry for the spamming... [23:19:15] *nod* it's OK! [23:19:24] (03PS17) 10Aarcos: Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 [23:19:40] That was a very helpful review tool [23:19:47] As, I'm sure, the tests will be. :) [23:20:05] passed now, please take another look ! [23:20:21] 'kay [23:27:29] Arright, here goes nothing [23:27:40] (03CR) 10MarkTraceur: [C: 032] Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [23:27:43] (03Merged) 10jenkins-bot: Add tests to MultimediaViewer [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98009 (owner: 10Aarcos) [23:27:51] * marktraceur looks for config to change [23:44:15] (03PS5) 10MarkTraceur: File usage dialog: Only reselect on focus, not on every click [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98128 (owner: 10Bartosz Dziewoński) [23:46:40] * marktraceur smacks head [23:46:51] (03CR) 10jenkins-bot: [V: 04-1] File usage dialog: Only reselect on focus, not on every click [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98128 (owner: 10Bartosz Dziewoński) [23:47:16] (03CR) 10MarkTraceur: "My bad, one sec" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98128 (owner: 10Bartosz Dziewoński) [23:55:05] (03PS6) 10MarkTraceur: File usage dialog: Only reselect on focus, not on every click [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/98128 (owner: 10Bartosz Dziewoński) [23:55:16] * marktraceur cruelly tricking jenkins [23:55:48] Mahahaha [23:55:59] * marktraceur mad scientist when it comes to QA stuff [23:56:26] aarcos: Your tests will now auto-run on patches we submit and will be run before merge of any MMV patch, grats [23:57:30] Awesome !, these are just [23:59:50] "smoke" tests, we "all" need to build on top of that so we increase the coverage and catch regression as early as possible. Glad to hear we have the infrastructre in place for continuous integration. Thanx for the review !