[08:13:48] hullo [08:53:57] phuedx: hey [09:00:21] hey d3r1ck [09:04:12] phuedx: whats up? [09:04:18] like to review some patches this morning? [09:08:42] is jdlrobson around? [09:09:53] d3r1ck: i'm trying to finish up a piece of work that i started on friday right now [09:10:13] add me as a reviewer on your patches though and i might be able to get round to looking at one or two [09:10:27] jdlrobson won't be around for a few hours [09:10:41] sorry -- not a few -- 7 hours [09:10:55] huggi: ok :) [09:11:02] phuedx: :) [09:12:28] phuedx: you are up, i added you. [09:12:36] d3r1ck: reading the scrollback, you should expect code review to be asynchronous and, generally speaking, have delays [09:12:57] people review in their own time and to different standards [09:13:41] phuedx: i understand. [09:13:59] phuedx: i will try to put that on my time table :) [09:14:58] the team that i'm on take ~1 day to review a patch that's submitted by a member of the team [09:15:11] but that's because all of our work is visible to the rest of the team [09:15:29] we try to make work from outside contributors as visible as possible [09:15:38] but it's hard sometimes [09:15:40] phuedx: wow, thats cool. [09:15:54] but review != merge [09:15:59] which is important to keep in mind [09:16:26] some patches are merged first time, but it's likely – and good! – that the patch won't be merged first time [09:17:04] you have to discuss problems, justify/refine your approach [09:17:10] talk with people about your change [09:17:14] which is why code review is great [09:17:29] so yeah, it takes time :) [09:20:02] phuedx: yeah. I perfectly understand you. [09:20:20] Florian told me something about that yesterday. [09:20:43] phuedx: i was kind of missing the point [09:20:53] phuedx: but thanks for bringing me up to speed. :) [09:25:41] phuedx: when you have time, just review :) [09:26:30] d3r1ck: ta :) [09:32:00] phuedx: what is ta? [09:32:32] "thanks" [09:32:47] british informal expression [09:36:57] phuedx: ohh, nice. ta too. [09:46:53] o/ [09:47:10] bmansurov: hey [09:47:44] hi [09:53:29] whats up bmansurov [09:53:31] ? [09:53:53] not much [09:58:56] nice. [10:18:56] phuedx: still working? :) [10:25:01] d3r1ck: yup [10:25:09] phuedx: ok [10:52:54] going to go lie down [10:53:02] the rest of my family has a cold [10:53:07] i suspect i'm coming down with it [10:58:07] phuedx|afk, the cure is near, a cup of british tea (and some medicine of course ;)) [11:01:58] phuedx|afk: i think bmansurov is saying the truth. [15:48:01] hey jdlrobson good morning! Do you want to bump sprint kickoff to tomorrow AM, since no Rob or Sam today? [15:52:15] ^ I'd be in favor of this as well, my internet provider is doing work this morning so my connection may drop unexpectedly [15:54:36] bmansurov: thinking about moving sprint kickoff to tomorrow AM, since we have no Sam or rob today (and jhobs is having internet issues). Waiting to hear from jdlrobson [15:54:49] kristenlans, sounds good [15:57:14] oh really? [15:57:27] ok, well yeh it would be good to have whole team :) [15:57:48] gotta love ISPs, I get no warning from them about maintenance and then my apartment complex has to email me this morning that they'll be working today and connections may drop :) [15:58:37] ok jdlrobson bmansurov jhobs rmoen phuedx|afk kaity I will move sprint kickoff to tomorrow. [15:58:49] sounds good [16:02:04] bmansurov: glad I checked the video call ;-) [16:02:13] me too ;) [16:02:22] i mean I'm glad you did [17:06:58] dbrant: fyi i've updated that q1 pageview sheet [17:07:51] dbrant: interestingly, link previews seem less spiky than page views [17:08:33] niedzielski: very interesting indeed. thanks! [17:46:59] niedzielski: ha, i was just thinking, "do we even need fragments? square doesn't" when i saw your comment [17:48:25] mdholloway: i'm very pro-View but i think we need a bit of a targeted convert blah to View patch to kick things off [17:49:06] mdholloway: not just fragments. for example, some of our more complex native views would benefit from being composed of multiple custom views instead of a monolith view inside the fragment [17:57:55] marxarelli: hi [17:58:04] niedzielski: hi [17:59:38] d3r1ck: \o [18:01:19] niedzielski: whats up man? [18:02:02] d3r1ck: just reviewing my last patch for the morning [18:02:07] you? [18:02:43] niedzielski: well just checked my email and gerrit bot did a merge to one of my patches [18:03:05] d3r1ck:nice [18:03:23] niedzielski: men, i need the code base to be in my finger tips [18:09:31] dbrant, aude, any thoughts about https://phabricator.wikimedia.org/T113827 [18:10:32] yurik: in the current patch, you can tap on the marker icon at the right of each list item, and it will highlight its corresponding marker on the map. [18:13:35] dbrant: i was checking out your db patch. it seems to work fine but i don't understand why we wouldn't want to use the loader in page fragment like we do in savedpagesfragment. [18:17:07] dbrant, thanks, i didn't notice it before. I think pin-click is a great feature, but i think item click should also do the pin click - makes it more obvious. Also, possibly change the color of the pin in the list to indicate that the pin has been highlighted [18:18:52] yurik: feel free to add that comment to the task! I'm asking designers to put together any remaining touch-ups for the UX of that feature. [18:19:29] niedzielski: for one thing, the loader seemed to be behaving inconsistently; it would sometimes return a null Cursor, which was causing this other crash: https://phabricator.wikimedia.org/T113766 [18:23:45] niedzielski: and also, in rearranging some of this code, it may shed additional light on the sqlite upgrade crash. [18:24:28] dbrant: do we know that the cursor is null and not the fragment? [18:25:34] dbrant: my real complaint is that we're doing the same thing two different ways [18:25:35] niedzielski: i don't see how the fragment can be null. it's always initialized with PageFragment.this. [18:35:08] dbrant|brb, updated https://phabricator.wikimedia.org/T113827 [19:45:28] dbrant|brb bearND mdholloway: did we want to cut a release today then? if so, are we hoping to put anything else in? [19:45:51] nothing from me [19:47:15] niedzielski: nothing from me as well. [19:50:53] niedzielski: let's go for it [20:03:14] mdholloway bearND: restbase link previews are still _off_ by default, right? [20:04:13] niedzielski: yep. [20:04:38] mdholloway bearND: did we want to turn those on for this release? [20:04:51] (to some small segment of betas) [20:05:41] niedzielski: not yet. We want https://phabricator.wikimedia.org/T104715 first [20:05:43] niedzielski: bearND: ...it would be nice, but i'd hate to rush it. [20:06:00] ^ good point, bearND [20:06:28] ok cool [20:21:15] mdholloway: we probably should push the latest service code to production [20:21:34] mdholloway bearND dbrant ok, release notes are cooking in the normal place. please give them a peek in the next hour or so [20:22:01] niedzielski: what's the url again? [20:23:21] (sent) [20:23:25] niedzielski: thanks [20:24:10] bearND: sure, should i just do it this time? [20:24:59] mdholloway: it's up to you. I've got my VM already fired up. [20:25:53] bearND: oh, i don't; i'd have to reboot my other laptop into ubuntu. if you want to do it that's cool with me. or i'm happy to join a hangout. [20:26:43] mdholloway: yeah, I can do it. I'm in the hangout already [20:27:00] ubuntu? more like uhomerunu [20:28:10] mdholloway bearND dbrant: heads up that this release will be a little different than usual for a couple reasons. notably, no version bump patch since we're already at 2.1.131 [20:54:39] jdlrobson: any evidence of problems mobile on the beta cluster since last Friday? We merged a patch runs a teeny bit more code everywhere, getting ready to deploy [20:55:08] CentralNotice patch, I mean [21:20:45] mdholloway bearND dbrant: hm, so i found one regression: https://img.bi/#/w4nu2WK!PgaehtPcahchyiqIiaBgnylEebaGmncIaLecrcIi . on API 21 (not 19, not 23, ...), when the subtitle is 2 lines and doesn't have a lead image, the offset is incorrect. everything else looks good [21:22:22] jhobs: how's the fix going? [21:22:29] AndyRussG: i've not seen anything suspicious [21:22:32] but i've not really been looking :) [21:23:11] jhobs: i need to step away for a bit so keen to get a patch for review around 3pm to give it a realistic shot at getting pushed out on the next train [21:23:40] AndyRussG: also good work :) less code is good :) [21:28:07] niedzielski: same on API 22 [21:29:07] niedzielski: mdholloway: hmm, that'll need to be fixed, i think [21:29:25] jdlrobson: heh this is a bit more code, requested by the performance team to regularly expire keep our localstorage items. Made it as compact as possible, kicks at idle times, works in teeny chunks... [21:29:51] jdlrobson: if you're interested: https://gerrit.wikimedia.org/r/#/c/240170/ :) [21:30:08] thanks also!! [21:30:33] mdholloway dbrant: i'll check it out, thanks [22:06:35] mdholloway bearND dbrant: fix proposed, testing welcome :) https://gerrit.wikimedia.org/r/241915 [22:06:56] k [22:06:58] niedzielski: i'll give it a spin [22:07:07] mdholloway: thanks! [23:05:04] niedzielski: fyi, it's merged now [23:09:36] bearND: thanks, just started a build [23:20:12] mdholloway|afk dbrant bearND: so if everything looks good, i'm going to cut a release. please make sure you're happy with the notes! :) [23:20:54] niedzielski: have you seen my comment on line 13? [23:21:22] bearND: oh, no i hadn't. the answer is yes [23:21:40] cool. thanks. Just wanted to make sure she knows [23:30:51] niedzielski: tweaked the release notes; good to go [23:31:08] dbrant:thanks! :) [23:31:52] dbrant: looks like we're over the 500 character limit now :| [23:32:20] there we go. [23:34:51] dbrant: i dropped the period for consistency and removed "new:" since the sentence made it clear that the feature was fresh [23:35:04] sure, ok [23:43:19] mdholloway|afk dbrant bearND: dang, i think i found another regression. if you go to the riacho fundo article, and tap "administrative region", the link is broken in link preview. it's because we're stripping the parens in the article title [23:44:47] niedzielski: this isn't a new issue, though, right? (not a regression since the last production version) [23:45:05] dbrant: i think it's new. let me double check [23:45:30] niedzielski: also note that we're only stripping parens in non-production. [23:45:55] dbrant: oh, i did _not_ note that :) [23:46:18] we should probably just remove that :/ [23:46:18] niedzielski: dbrant: hmmm, when I go from the link preview to the article it says it doesn't exists [23:46:33] yeah, it's because of parens stripping from the link [23:47:22] dbrant: you're right. not a regression as compared to the last beta [23:50:48] dbrant bearND mdholloway|afk: since this isn't a regression, are we cool with moving forward? i didn't spot anything else [23:51:00] niedzielski: yep [23:51:07] cool [23:51:38] yayy [23:51:54] niedzielski: dbrant is there a task for this issue? [23:52:11] there almost certainly is. looking... [23:52:55] bearND: https://phabricator.wikimedia.org/T96871 [23:53:42] anyone wanna confirm for me that all's good for iPhone users following a CentralNotice deploy that went out a little while ago pls? https://en.wikipedia.org/wiki/Main_Page?country=IL [23:54:17] dbrant: thanks. Wow, that one is old. Should bump it up, tho [23:54:42] jdlrobson: ^ ? :) [23:55:04] A banner should show for that URL, then if you click the close button and reload, it shouldn't