[00:03:09] phew finished a brief kids' homework multitask..... [00:04:17] fyi, I'm working on mobile niceness [00:04:40] cool!!! [00:04:57] I'm doing the JS patch that we've all been waiting for... [00:05:51] O_o [00:05:55] \o/ [00:05:57] * [00:06:05] ... [00:06:31] do the automated browser tests include mobile? [00:06:45] (PS2) Awight: restore new banner choice modules [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173198 [00:06:54] AndyRussG: trying to figure out how we can do that... [00:07:08] K thanks!!! :) [00:07:19] AndyRussG: actually, that's easy. I'm trying to figure out QUnit coverage for mobile... [00:07:25] that's not easy ;) [00:07:35] hummmm [00:15:13] (CR) Jdlrobson: [C: -1] "Apart from the rogue comment it looks okay (but havent quite tested)" (1 comment) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173198 (owner: Awight) [00:16:30] (CR) Awight: restore new banner choice modules (1 comment) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173198 (owner: Awight) [00:17:45] (PS3) Ejegg: test fixtures do not pollute cn_known_devices table [extensions/CentralNotice] (wmf_deploy) - https://gerrit.wikimedia.org/r/173193 (owner: Awight) [00:18:00] ack, wrong branch [00:18:28] (CR) jenkins-bot: [V: -1] test fixtures do not pollute cn_known_devices table [extensions/CentralNotice] (wmf_deploy) - https://gerrit.wikimedia.org/r/173193 (owner: Awight) [00:18:35] yeah, did I do that? [00:19:24] i guess maybe? I was just adding some stuff to the refs I pulled from gerrit [00:19:29] will swap that over [00:19:31] thx! [00:19:36] also :( the lights are all red [00:20:10] odd, it was qunit failing that patchset [00:20:38] (CR) Robmoen: [C: 1] "LG2M, tested without errors :)" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173198 (owner: Awight) [00:21:00] (PS1) Ejegg: test fixtures do not pollute cn_known_devices table [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173211 [00:22:33] my addition was kinda silly, using addFixtures and removeFixtures from a couple other test classes just to ensure the desktop device [00:23:46] wat :) [00:24:36] ejegg: maybe we should have a CN base test class, and make fixture->ensureDesktopDevice public [00:24:59] (CR) Awight: [C: 1] "Additions look good." [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173211 (owner: Ejegg) [00:26:01] (CR) Ejegg: [C: 2] test fixtures do not pollute cn_known_devices table [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173211 (owner: Ejegg) [00:26:27] awight: sounds like a good TODO [00:26:32] k [00:26:51] also... that should just freaking happen. I should debug the sqlite install. [00:30:32] (Merged) jenkins-bot: test fixtures do not pollute cn_known_devices table [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173211 (owner: Ejegg) [00:33:39] (PS5) Ejegg: Tests for BannerAllocationCalculator [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172958 (owner: Awight) [00:33:46] (PS1) Awight: Reuse $wgCentralDBname for the Choice infrastructure [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173218 [00:36:18] hehe, https://gerrit.wikimedia.org/r/#/c/173119/2/includes/skins/SkinMinervaBeta.php,unified [00:36:51] roundabout way of nuking it, i guess [00:37:15] at least it worked! [00:37:32] wait, how does that just pertain to beta? [00:37:44] oh... skinminervabeta [00:37:47] nvm [00:37:47] yah [00:40:26] ComparisonUtil's exceptions vs assertions are a bit odd, but they'll raise the red flag just as high [00:40:33] AndyRussG: 16:38 < bd808> awight: The tshirt signup form is at http://etherpad.wikimedia.org/p/IBrokeWikipediaList [00:41:09] that's bad to the bone [00:41:17] ejegg: you want one? :p [00:41:18] nice... [00:41:21] eek [00:41:52] i guess i did bust things all over with that caching / json dependency deploy [00:42:11] ooh! Sign up while supplies last, a lot of people qualify :) [00:42:43] ooh, debug=true in ios app bits url! must have been a pain to get a fix out [00:43:02] (oo) [00:43:23] I'll take blame but as for the credit, especially for fixing, it's definitely awight's [00:43:35] I think we each get a shirt... [00:44:02] ...or parts of one? [00:44:30] I'll take it up to where it says "I broke" [00:44:48] your family will probably not like the results ;) [00:44:51] belly shirt? [00:46:18] * AndyRussG madly writes javascript [00:47:48] AndyRussG: ejegg: you guys are more than welcome to deploy after I leave (in 1/2 hr) [00:47:58] heheh [00:48:03] just ahhh, stick around in IRC for a couple of hours after that [00:48:12] I find it sooo soothing to write code without a million $'s [00:48:18] seriously. [00:48:29] it's like nirvana [00:48:29] yup [00:48:34] did you see there's this blasted new convention however of $-prefixing jQuery variables? [00:48:45] screw that hungarian revival notation [00:48:48] ah yeah I do that [00:48:54] gross! [00:49:02] have some snot! [00:49:14] next i know, people will be m- prefixing members and s- prefixing strings [00:49:15] I mean jQuery objects [00:49:19] $not? [00:49:20] terrible habit. [00:50:06] (PS3) Awight: Tests for BannerAllocationCalculator [extensions/CentralNotice] (wmf_deploy) - https://gerrit.wikimedia.org/r/173194 [00:50:26] ...dunno about you folks, but I'm more like, get things working and tested and tested and tested and then we'll see if we're still allowed to deploy... [00:50:39] awight: I moved that back onto master: https://gerrit.wikimedia.org/r/#/c/172958 [00:51:01] ooh thx [00:51:13] or we could deploy by controlling a cluster with our brainwaves [00:51:14] (Abandoned) Awight: Tests for BannerAllocationCalculator [extensions/CentralNotice] (wmf_deploy) - https://gerrit.wikimedia.org/r/173194 (owner: Awight) [00:51:31] (Abandoned) Awight: test fixtures do not pollute cn_known_devices table [extensions/CentralNotice] (wmf_deploy) - https://gerrit.wikimedia.org/r/173193 (owner: Awight) [00:51:46] yikes! [00:51:50] watch out for transients [00:52:30] you mean random qunit failures? [00:53:36] or drifters with tinfoil interfering with our brainwave deployments? [00:55:53] the latter. thinking about dinner or finding change in the sofa [00:55:58] real thoughts [00:56:43] (PS1) AndyRussG: WIP Choose banner on client [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173220 [00:57:16] (CR) jenkins-bot: [V: -1] WIP Choose banner on client [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173220 (owner: AndyRussG) [00:57:19] so far only has one measly function [00:57:34] for ur testing pleasure [01:00:00] woohoo, client-side code! [01:00:58] just a smidge [01:04:40] (PS1) Ssmith: Make navbar px-perfect [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173224 [01:04:43] (CR) jenkins-bot: [V: -1] Make navbar px-perfect [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173224 (owner: Ssmith) [01:06:46] (CR) Ejegg: [C: 2] Tests for BannerAllocationCalculator [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172958 (owner: Awight) [01:08:23] (PS3) Ssmith: Extend ChartJS and make selection chart dynamic [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173190 [01:08:27] (CR) jenkins-bot: [V: -1] Extend ChartJS and make selection chart dynamic [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173190 (owner: Ssmith) [01:21:46] (PS1) Awight: WIP refactor test fixtures in json so we can reuse from QUnit [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173225 [01:22:17] (CR) jenkins-bot: [V: -1] WIP refactor test fixtures in json so we can reuse from QUnit [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173225 (owner: Awight) [01:22:34] ejegg: AndyRussG: see you in a week! [01:22:44] have a good one! [01:22:57] awight: bye!!! enjoy! [01:23:01] the hustle was fun, sort of :) [01:23:02] and thanks!!!!!!!!!! [01:23:12] unforgettable [01:23:25] ;) yeah let's not do this again [01:23:48] it would have been great if not so compressed... [01:24:05] squished [01:24:12] hmm oh well [01:24:29] you may enjoy your new buckets when you return [01:24:33] (vacation?) [01:24:35] yah! [01:24:46] cool! [01:24:47] yep, taking the runt to see her great-gp's [01:24:56] Fantastic [01:25:09] enjoy the down time :) [01:25:40] um... anything else... you might want to define the risks of doing this deployment on Monday. I think it's manageable. but it will help if we are all clear about what we might be getting into. [08:19:18] (PS2) Awight: refactor test fixtures as json [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173225 [08:19:54] (CR) jenkins-bot: [V: -1] refactor test fixtures as json [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173225 (owner: Awight) [08:23:24] (PS3) Awight: refactor test fixtures as json [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173225 [08:28:28] (PS1) Awight: DO NOT MERGE octopus branches together for development [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173243 [09:09:17] (Restored) Hashar: Jenkins job validation (DO NOT SUBMIT) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/46700 (owner: Hashar) [09:09:24] (CR) Hashar: "recheck" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/46700 (owner: Hashar) [09:48:47] (PS1) Awight: QUnit tests for client banner allocations [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173249 [09:49:24] (CR) jenkins-bot: [V: -1] QUnit tests for client banner allocations [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173249 (owner: Awight) [09:49:28] (PS2) Awight: WIP QUnit tests for client banner allocations [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173249 [09:50:02] (CR) jenkins-bot: [V: -1] WIP QUnit tests for client banner allocations [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173249 (owner: Awight) [10:00:07] (CR) Awight: "Krinkle pointed out that this is not safe to run under Jenkins, cos the asynchronous creation of tests might be cut short. There are some" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173249 (owner: Awight) [10:13:26] (PS1) Awight: WIP Floundering attempt to protect async, dynamic test creation from QUnit cleanup [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173253 [10:14:07] (CR) jenkins-bot: [V: -1] WIP Floundering attempt to protect async, dynamic test creation from QUnit cleanup [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173253 (owner: Awight) [15:30:31] (PS1) Cmcmahon: QA: add mobile URL target for Jenkins build [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173282 [15:34:49] (PS2) Cmcmahon: QA: add mobile URL target for Jenkins build [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173282 [15:36:19] (PS3) Cmcmahon: QA: add mobile URL target for Jenkins build [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173282 [15:38:58] (CR) Zfilipin: [C: 2] QA: add mobile URL target for Jenkins build [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173282 (owner: Cmcmahon) [15:39:30] (Merged) jenkins-bot: QA: add mobile URL target for Jenkins build [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173282 (owner: Cmcmahon) [16:35:52] (PS2) AndyRussG: WIP Choose banner on client [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173220 [16:36:26] (CR) jenkins-bot: [V: -1] WIP Choose banner on client [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173220 (owner: AndyRussG) [16:36:48] (CR) AndyRussG: "Just rebasing on patch to restore modules" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173220 (owner: AndyRussG) [16:38:14] (PS2) Awight: WIP Floundering attempt to protect async, dynamic test creation from QUnit cleanup [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173253 [16:38:48] (CR) jenkins-bot: [V: -1] WIP Floundering attempt to protect async, dynamic test creation from QUnit cleanup [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173253 (owner: Awight) [16:45:15] (PS3) Awight: Protect async tests from QUnit cleanup [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173253 [16:45:50] (CR) jenkins-bot: [V: -1] Protect async tests from QUnit cleanup [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173253 (owner: Awight) [16:47:51] (PS4) Awight: Protect async tests from QUnit cleanup [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173253 [16:50:31] (PS3) Awight: QUnit tests for client banner allocations [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173249 [16:51:16] (PS4) Awight: WIP QUnit tests for client banner allocations [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173249 [16:51:30] AndyRussG: you might want to squash this patch into yr current work: https://gerrit.wikimedia.org/r/#/c/173249/ [16:52:06] Hi awight! ...k lemme se :) [16:52:07] Also, I think this is both safe and useful to merge: https://gerrit.wikimedia.org/r/#/c/172665/ [16:52:37] (PS4) Ssmith: Extend ChartJS and make selection chart dynamic [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173190 [16:52:46] (CR) jenkins-bot: [V: -1] Extend ChartJS and make selection chart dynamic [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173190 (owner: Ssmith) [16:53:51] (Abandoned) Awight: Protect async tests from QUnit cleanup [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173253 (owner: Awight) [16:54:04] cool... [16:54:51] yah! I'm happy that worked out... [16:55:08] QUnit is fun! [16:55:39] yes... it was especially fun trying to declare my tests from an asynchronous callback :p [16:56:29] see this nonsense, where I attempted it: https://gerrit.wikimedia.org/r/#/c/173253/2/tests/qunit/ext.centralNotice.bannerController.lib/bannerController.lib.tests.js,unified [16:57:55] n e way, if we merge the basic QUnit patch, u won't need an octopus merge to develop using the bannercontroller.lib followon tests [16:58:25] What's an octopus merge? [16:58:50] Have you run mw qunit tests before? It's just, set $wgEnableJavaScriptTest=true and then browse to Special:JavaScriptTest/qunit?module=ext.centralNotice.bannerController.lib [16:59:09] Also what's the fundamental difference here https://gerrit.wikimedia.org/r/#/c/173253/2/tests/qunit/ext.centralNotice.bannerController.lib/bannerController.lib.tests.js,unified and here https://gerrit.wikimedia.org/r/#/c/173249/4/tests/qunit/ext.centralNotice.bannerController.lib/bannerController.lib.tests.js? [16:59:34] Yes I have tho I didn't remember exactly that detail... [16:59:40] I may be exaggerating about the octopus merge, but it's when you merge many feature branches together in one commit [17:00:49] Ah, the difference is very unfortunate: the nice way would have been to declare a test for each fixture. Instead, we have to run all assertions under one test. [17:01:04] Hum [17:01:09] Krinkle was helping me with that, but currently I'm blocked, so I went with the workaround. [17:01:20] Sounds like a good plan! [17:02:12] BTW thank you for continuing to plow ahead on this, I was imagining you + extended family on a flight to the Mayan Riviera... or is there wifi on your plane? :) [17:02:51] nah, just New England [17:02:52] awight: aside from todo's and commented out code, just fyi: space after 'function', and 'forEach' is not available in browsers we require. [17:02:56] we'll come back with frost tans [17:03:19] Krinkle: oh thanks! [17:03:30] Krinkle: we can do $.forEach, no? [17:03:33] $.each [17:03:40] right that [17:03:48] Krinkle: would be nice if that were covered by jslint rules... [17:04:06] I wonder if it's more expensive than a plain for... now that I've got mobile on my mind... [17:04:20] responseData is a foreign object in this context. forEach is just a property of some random object. Can't be statically determined. [17:04:29] meh this is just the test, so performance is not so important [17:04:35] ah true [17:04:43] And new ES5 browses do have it, and we have a polyfill. It's just a matter of wanting to support newer browsers only for your feature. [17:04:57] Yeah it has to be broadly supported [17:05:10] tests ideally would not require newer browsers than the feature at hand, so we can run tests in them :) [17:05:14] There was another bit where I was gonna do $.each but I held back becuase I thought a plain for would be sturdier and a bit faster [17:05:24] Hmmm right [17:05:43] Maybe headless test browser can be configured as to which JS features to allow? [17:07:08] awight: I guess not upstate NY or northern VT? [17:07:22] (PS5) Awight: QUnit tests for client banner allocations [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173249 [17:07:48] luteium replication is restored, it's catching up [17:10:29] AndyRussG: fyi, the browser tests are running on the mobile target, now. [17:10:46] awight: fantastic! [17:11:17] Very crude though. Plus, they rely on CN fixtures existing on betawiki. And, as u know, depend on the ?random= override to pass. [17:11:42] ok I thinky my time here is limited. I hear snoofling from the next room :) [17:11:51] s/y// [17:12:05] oh snoofing! [17:12:29] awight: so your impressive footwork has now landed us here somewhere? https://integration.wikimedia.org/ci/view/BrowserTests/view/-All/ [17:13:10] Or is it rather that we're ready to go there? [17:13:22] Krinkle: hashar mentioned an interesting idea yesterday, that we might want to run QUnit tests on different skins, and with device=mobile. We ran into an interesting bug, where the fallback skin is missing the siteNotice div that CN requires. [17:13:31] AndyRussG: err, we should be there, lemme get the URL [17:13:47] :D [17:14:31] https://integration.wikimedia.org/ci/job/browsertests-CentralNotice-en.wikipedia.beta.wmflabs.org-linux-chrome-sauce/ [17:14:47] awight: Code shouldn't have anything skin specific in it in the first place. Any skin specific code should either use abstraction layers like mw.util, or be loaded as skinScripts/skinStyles. Elements you hook into should be outputted by the extension or part of the standard parser output. And mocked in tests of course, since it is a unit test, and on Special:JavaScriptTest is none of your eleme [17:14:47] nts anyway. [17:15:03] AndyRussG: You can see, it will randomly pass 1/9 of the time :) https://integration.wikimedia.org/ci/job/browsertests-CentralNotice-en.wikipedia.beta.wmflabs.org-linux-chrome-sauce/3/console [17:16:31] K iiinteresting [17:16:51] Krinkle: I did mock the element to workaround that issue. However, what would you suggest for the production code? It needs to attach somewhere, so that's pretty much a contract on abstract skins. For example, https://gerrit.wikimedia.org/r/#/c/172870/ [17:17:11] s/abstract/every/ [17:17:45] We wouldn't want to just prepend a div#siteNotice... [17:17:49] awight: Right. Adding it to the fallback masks the problem, it doens't solve it and tests should mock it regardless of whether it works. [17:18:03] So that the page isn't left behind in a dirty state afterwards. [17:18:09] Yes, in the test that's exactly what I did [17:18:28] should be inside qunit-fixture (or left detached if the tested function takes a node reference) [17:18:32] But I was saying, in production, the skin just... needs to have a siteNotice div, if we ever expect to display notices. [17:18:39] Yes [17:18:47] and if there is none, fallback to displaying nothing. [17:19:02] The skin decides whether it should be displayed by having the element. I guess that's sort of an invitation. [17:19:45] awight: yeah I have bring that topic on a bug report yesterday [17:20:09] awight: Krinkle: https://bugzilla.wikimedia.org/show_bug.cgi?id=73377#c17 [17:20:21] we had an issue of some javascript removal from Mantle breaking a bunch of other repositories [17:20:25] Krinkle: good. Yeah that's how I feel, as well. Removing or not displaying the siteNotice div is the contract for "hide CN" [17:20:34] so in theory we could have all extensions having qunit jobs to share a common job [17:20:43] zuul-cloner can let us do that [17:21:05] something like: zuul-cloner mediawiki/core mediawiki/vendor mediawiki/extensions/{Mantle,MobileFrontend,Flow,Echo} .. [17:21:16] then run the qunit tests and hope they works well together :D [17:23:22] (CR) Krinkle: [C: -1] Basic QUnit tests (2 comments) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 (owner: Awight) [17:26:22] (CR) AndyRussG: "Other than the inline comment, LGTM! ;)" (1 comment) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173198 (owner: Awight) [17:26:58] (PS12) Awight: Basic QUnit tests [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 [17:27:00] (CR) jenkins-bot: [V: -1] Basic QUnit tests [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 (owner: Awight) [17:28:30] grr. [17:29:34] (PS13) Awight: Basic QUnit tests [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 [17:29:45] (CR) Awight: "PS 13: rebase" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 (owner: Awight) [17:30:06] (CR) jenkins-bot: [V: -1] Basic QUnit tests [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 (owner: Awight) [17:31:46] hehe, maybe we should modify our json parsers to accept comments and trailing commas [17:31:57] (CR) Krinkle: Basic QUnit tests (6 comments) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 (owner: Awight) [17:32:00] (PS14) Awight: Basic QUnit tests [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 [17:32:33] (CR) jenkins-bot: [V: -1] Basic QUnit tests [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 (owner: Awight) [17:33:11] awight: globals is not an array, predef is. [17:33:45] awight: the json parser accepts comments, and in jshintrc these are encouraged. [17:33:53] however trailing comma's are invalid json. [17:34:26] they have speciall meaning in older javascript engines so basically can't be added/tolerated. [17:39:01] Krinkle|detached: wait, I was poking fun at the recent change to FormatJson. But the jshint parser actually does tolerate them? [17:39:09] \o/ [17:41:10] (PS15) Awight: Basic QUnit tests [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 [17:41:42] (CR) jenkins-bot: [V: -1] Basic QUnit tests [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 (owner: Awight) [17:43:47] * K4-713 waves [17:43:49] Hey, it's K4-713!! [17:44:08] I'm not really here. I just really wanted to know how yesterday went. [17:44:29] Yeah... Mmm tl;dr: deadline missed... [17:44:39] Hah, that's totally fine. [17:44:45] Was it painful? [17:44:49] Yes [17:44:59] Dang. [17:45:26] A small deploy of some supposedly no-op pieces caused some mobile breakage, limited I think to the planet earth, fortunately [17:45:49] Oh, well that's good, then. Relatively contained. :) [17:45:52] Got fixed quickly [17:46:16] I believe that. [17:46:25] The blame is mine and the credit for quick fixing go to awight and the mobile team for noticing quickly too [17:46:51] Oh, well, you know they say you don't really work here until you completely ruin something for a few minutes. [17:47:08] I took down all the projects completely for three minutes once. [17:47:37] Hmmm oh yes I have been told that it's OK... [17:47:53] Those saying it are serious. It totally happens. [17:47:54] May have been longer than 3 minutes! Not sure because caching compexities [17:47:56] (PS16) Awight: Basic QUnit tests [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 [17:47:58] (PS1) Awight: Clean up jshintrc and namespace closures [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173314 [17:48:21] Well, I'm glad it got sorted out. [17:48:28] Heh yes me to!!! [17:48:36] awight: I thought you said you weren't working today. [17:48:45] Not working looks a lot like working. [17:48:47] I'm not here either. [17:48:47] awight and ejegg did some fantastic work on tests and now we even have failing sauce labs tests! https://integration.wikimedia.org/ci/job/browsertests-CentralNotice-en.wikipedia.beta.wmflabs.org-linux-chrome-sauce/ [17:48:54] awight: :) [17:49:00] that one is kind of the most useless test though [17:49:12] the only nice part is that we can have it for window$, mobile etc [17:49:15] That's pretty huge, though. [17:49:33] Yay tests. [17:49:38] I'm more excited about... https://gerrit.wikimedia.org/r/#/c/173249/ [17:49:53] :) [17:49:56] it lets us run the same test cases through the legacy server allocation and through the client stuff [17:49:59] yeah there have been some really good advances [17:50:14] K4-713: in other words... in case it wasn't obvious... [17:50:27] we're totally gearing up for a Monday deploy attempt :-! [17:51:01] Mmmmmmmmmhm. [17:51:07] ("attempt" including hoping to be allowed to...) [17:51:13] K4-713: I don't want u to have to take the role of nay/yaysaying, although I'm sure you would be happy to :) -- I was thinking, we could all chat about the potential ways this will break everything subtly, though. [17:51:54] Eh. [17:52:15] My line of thought so far is, if impressions numbers don't drop, and we aren't serving obviously wrong banners... subtle things might not actually hurt? [17:52:16] I feel kinda not wanting to deploy everything until lots of tests are passing and also only if everyone is very comfortable with the code and the timing... [17:52:33] also, what AndyRussG said :) [17:52:35] It may be the post-op pain killers talking, but there's only one way to really know. [17:52:47] * awight revs bicycle loudly [17:52:58] :) [17:53:54] Anyway... typing from this position is awkward and difficult, and I can't really move. [17:53:58] In any case today and this weekend I don't think there's anything else I should be working on, so I can go on... [17:54:06] * awight just blinked at Jenkins speaking Spanish to me [17:54:12] I just wanted to know how things were, you know... going. [17:54:22] sorry for inducing bad dreams! [17:54:27] Nah. [17:54:42] I figured if anything went wrong enough, I'd have heard about it. [17:54:51] oh, on the bright side. there's a sign-up sheet for *actual* "i broke wikipedia.. but then i fixed it" T-s [17:54:54] woot! [17:55:35] I need one of those. [17:55:41] K4-713: We can sign u up for a t-shirt too then :) [17:55:47] Rad. [17:56:09] Hope you feel better quickly!! :) [17:56:10] (CR) Awight: Basic QUnit tests (4 comments) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172665 (owner: Awight) [17:56:54] In about 7 minutes I have to go to a lunch engagement, that I decided not to cancel because it will contribute to my emotional well-being [17:56:59] Oh, I feel basically fine. I just can't move around or sit up or anything. [17:57:07] And ice packs. So very many ice packs. [17:57:15] Uuff [17:57:42] I think Sage Ross does IRC with voice recognition and syntehsis from his phone [17:57:55] Ha! [17:58:12] My phone keeps trying to get me in trouble, though. [17:58:36] AndyRussG: and your gastro- well-being. You easterners seem to be eating air [17:58:50] I believe I have taught it too many impolite words. [17:58:57] K4-713: lol [17:59:13] K4-713: hehe, like editing your animal crossing spreadsheets and mailing it out to coworkers based on curse-recognition [17:59:22] "fucking boss" [17:59:23] whoops! [17:59:32] bahaha [18:00:09] awight: actually I'm going for Thai... with some old friends who I have hardly seen or haven't seen since I moved back here [18:00:10] I remember earlyish cell phones, where the first minute of every conversation was both parties cursing at reception, then apologizing for said curses [18:00:16] awesome! [18:00:21] K4-713: go away! [18:00:25] okay [18:00:26] AndyRussG: u too :p [18:00:32] I'll go first, then [18:00:34] awight: you three! [18:00:37] :) [18:00:47] * K4-713 salutes [18:00:51] :) [18:00:56] * awight puts on a good limp [18:00:58] aww [18:02:11] * AndyRussG , relieved that things seem OK, goes to put on shoes [18:02:29] awight: if I don't talk to you before you take off, have a great trip! [18:04:23] tty soon, all [18:05:31] (CR) Ejegg: [C: 2] Clean up jshintrc and namespace closures [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173314 (owner: Awight) [18:07:17] (PS3) Awight: restore new banner choice modules [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173198 [18:07:52] (CR) Awight: restore new banner choice modules (1 comment) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173198 (owner: Awight) [18:10:28] (CR) Awight: "thanks!" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173282 (owner: Cmcmahon) [18:19:20] (PS1) Glaisher: Show a friendly error page when requested banner does not exist [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173323 (https://bugzilla.wikimedia.org/54180) [18:20:12] (CR) Glaisher: "Attempted to fix it. Ib1a770d7c9b56a045fa1b11104bd88e831d6931d" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/157484 (https://bugzilla.wikimedia.org/54180) (owner: Ori.livneh) [18:44:29] (PS7) Awight: WIP BannerChoicesResourceLoaderModule test [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172882 [18:45:02] (CR) jenkins-bot: [V: -1] WIP BannerChoicesResourceLoaderModule test [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/172882 (owner: Awight) [18:45:20] (CR) Ejegg: [C: -1] "Couple suggestions inline" (4 comments) [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/171982 (owner: Ssmith) [19:46:56] (CR) Ejegg: "Extending w/o altering lib is great. Good to go back to single observable for each boundary. Might want to look at chart's .update(). P" (1 comment) [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173190 (owner: Ssmith) [19:50:30] ejegg some of the updates are out of order, sorry about that :-/ [19:50:50] I did play with .update() [19:50:57] but it didn't work [19:53:34] oh, gotcha [19:54:32] is https://gerrit.wikimedia.org/r/171982/ still valid / in progress? [20:02:47] hey, how about we add a command-line debug switch that serves stuff out of /src and disables the auth? That way we don't have to worry about checking in changes to routes [20:08:22] (PS2) Ejegg: Show a friendly error page when requested banner does not exist [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173323 (https://bugzilla.wikimedia.org/54180) (owner: Glaisher) [20:09:20] (CR) Ejegg: [C: 2] "Looks great! I just changed a tiny bit of whitespace." [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173323 (https://bugzilla.wikimedia.org/54180) (owner: Glaisher) [20:10:00] (Merged) jenkins-bot: Show a friendly error page when requested banner does not exist [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173323 (https://bugzilla.wikimedia.org/54180) (owner: Glaisher) [20:56:55] (CR) Ejegg: [C: -1] "I think we only need to change the .modules.php file." (1 comment) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173107 (owner: Awight) [21:04:44] AndyRussG pizzzacat standup? [21:04:54] Coming!! [21:11:54] atgo sorry, was in the middle of a dumb rebase [21:12:01] it happens [21:12:08] my standup is that I'm doing dumb rebases [21:12:13] ha [21:12:38] and working on Dash Big English / SQL string correction / validation [21:12:48] rebases make me itchy [21:14:10] good thing awight isn't here to make fun of me about making them super hard [21:19:44] ejegg: would you have still a bit of time for code review this afternoon? what time are you around until? [21:19:55] thx in advance :) [21:23:31] AndyRussG: definitely! [21:23:41] fantastic :) [21:23:51] I'll be online till 7:30-8 eastern [21:24:01] ejegg: OK excellent thanks :) [21:28:24] (CR) Ssmith: "these were fixed in a later patch: https://gerrit.wikimedia.org/r/#/c/173064/2" [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/171982 (owner: Ssmith) [21:32:00] (PS1) Ejegg: Add debug command line switch [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173425 [21:33:16] (CR) Ejegg: [C: -1] "Makes things less perfect in FF." (1 comment) [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173224 (owner: Ssmith) [21:37:36] Jeff_Green did you see megan's email about the db repairs fro yesterday? [21:38:01] no, lemme look [21:40:09] replied [21:41:31] thanks :) [21:41:45] np. sorry I forgot to report back! [21:42:40] no worries [22:02:26] (PS1) Ejegg: Fix input direction for rtl langs when sitedir=ltr [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/173435 [22:21:47] (CR) Ejegg: [C: 2] "I like this version! Works locally for me with useformat=mobile." [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173198 (owner: Awight) [22:23:08] (Merged) jenkins-bot: restore new banner choice modules [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173198 (owner: Awight) [22:25:47] ejegg: cool, thanks! [22:47:59] ejegg: do you understand why ext.centralNotice.bannerController.mobile doesn't need the "target" property set? Is it because it's default loaded from onSkinMinervaDefaultModules? [22:48:48] I'm also having no problems with useformat=moble now, on the current master [22:49:37] also agreed that this doesn't make sense: https://gerrit.wikimedia.org/r/#/c/173107/6/includes/CNBannerChoiceDataResourceLoaderModule.php [22:55:07] (PS3) AndyRussG: WIP Choose banner on client [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173220 [22:55:46] (CR) jenkins-bot: [V: -1] WIP Choose banner on client [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173220 (owner: AndyRussG) [22:56:07] AndyRussG: let me see [22:56:11] (CR) AndyRussG: "Rebased again" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173220 (owner: AndyRussG) [22:56:29] thanks! [22:57:20] AndyRussG: take a look at the bottom of CentralNotice.modules.php [22:58:21] ejegg: you mean the array_merge_recursive with $wgResourceModules[ 'ext.centralNotice.bannerController' ]? Yeah I saw that :p [22:58:49] Still not getting the target property tho [22:58:49] Ah, cool [22:59:52] I'm just wondering if for consistency we should add target => desktop on one and mobile on the other, and whether it matters in any way other than just for consistency/explicitness [22:59:56] I think 173198 was a replacement for 173107, the one that you agreed didn't make sense [23:00:46] huh, let me see if I get the target property set. Where are you checking? [23:01:46] pff I started by putting a breakpoint at getTarget in ResourceLoaderModule [23:02:12] But that doesn't always get called, it must be grabbing the property directly somewhere [23:02:52] it's definitely loading device.js for me with useformat=mobile [23:02:53] The only call to getTargets() that I could find is in OutputPage::filterModules() [23:03:35] did you pull in the recently-merged re-enablement: https://gerrit.wikimedia.org/r/173198 [23:03:42] Yeah! and also running our new CN modules with useformat=mobile [23:04:16] There's another getTargets() in ResourceLoaderStartUpModule [23:04:41] ejegg: I did rebase on top of that, yes thanks for +2'ing :) [23:05:08] Hmmm [23:05:46] Didn't get there, but I did try breakpointing directly in getTargets [23:06:16] Though I wouldn't describe XDebug as 100% reliable, I have found it occaisonally to be flaky [23:07:18] Huh, it's working for me. I set the breakpoint in ResourceLoaderStartUpModule, and it gets [ 'mobile' ] back from getTargets [23:07:37] On our CN modules? [23:07:54] On the bannerController.mobile module specifically [23:08:52] Hmm so maybe that's added by the mobile skin when the onSkinMinervaDefaultModules is called [23:09:20] If you disable mobile view does it still call it and get that? [23:09:28] Let me check [23:10:09] thx! [23:13:55] bah, xdebug isn't stopping anyplace now [23:14:30] mobile or otherwise [23:14:43] yurp [23:15:06] let's see if restarting browser + ide fixes that [23:15:18] (that's short for "Yep, mobUl skin deRP" [23:15:22] ) [23:18:15] well, fnord. even restarting apache didn't fix xdebug [23:19:58] ok, gonna relocate from this cafe. back soon [23:20:07] K :) [23:44:41] ejegg: check out $wgMFMobileSpecialPageResourceBoilerplate in MobileFrontend.php [23:46:50] ok [23:47:16] That may be what's adding it... [23:48:05] you think? [23:48:43] No, that's only used in setting up mobileFrontend's own modules [23:49:05] I'm pretty sure CN's .modules.php sets the target just fine [23:49:36] it seems to work but where doez it do it? [23:49:59] it's that array_merge_recursive [23:51:12] I don't think so, 'cause none of the arrays that get merged in there have a 'target' key, no? [23:51:40] targets, but not target [23:52:05] ejegg: I am blind [23:52:08] brrhggggg [23:52:21] sorry to have wasted your time with that [23:52:24] :( [23:52:30] oh, no worries [23:53:51] heh, maybe I should worry... 8p but thank you for being understanding :) [23:54:40] And I guess the non-mobile module doesn't need 'desktop' among its targets...? [23:54:56] (CR) Ejegg: [C: 2] "Digging the reusability and code / data separation!" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/173225 (owner: Awight) [23:55:07] * AndyRussG evil-ly looks for more rabbit holes