[16:06:02] good morning Jeff_Green! [16:07:32] hey atgo [16:07:41] how are you doing? [16:07:50] alright--you? [16:25:10] good! i'm on your coast [16:25:31] Jeff_Green: i just wanted to see if you'd gotten what you needed to get that civi account set up? [16:26:34] atgo: ohrlly. enjoying the fine weather!? [16:26:49] ha it's actually warm in nyc today! high of like 66 [16:27:07] yeah, you just missed the cold snap [16:28:11] atgo still haven't heard back from the user [16:28:22] at least, unless I missed it somehow :-( [16:28:43] no... ccogdill any news on heather's cell? [16:28:59] yeah she replied to the thread yesterday [16:29:31] ah, but looksl ike she didn’t cc anyone [16:29:41] I’ll forward to you Jeff_Green [16:30:05] ok [17:03:50] Jeff_Green: ccogdill lmk when that's all done and i'll mark it off my list [17:36:38] AndyRussG: hi! I'm just catching up on stuff, and wanted to double-check that you would be amenable to flipping the CN configuration today, once meganhernandez and K4-713 agree on the timing? [17:37:00] Hey awight! How's it going? [17:37:02] It sounds like nobody has noticed issues with the mediawiki.org rollout... [17:37:14] howdy awight and AndyRussG [17:37:15] AndyRussG: great! twas quite nice to have a week without a computer [17:37:16] I was just thinking about the timing... [17:37:26] Hey ejegg :) [17:37:37] (although all my relatives had computer maintenance stuff :-$) [17:37:41] awight: heh, does that exist? I thought it was physically impossible [17:37:47] hehe see ^^ [17:37:54] right... [17:38:00] but no laptop!! [17:38:09] I slept like a baby [17:38:19] heh, right... [17:38:37] No more nightmares about allocation precentages? [17:38:50] hehe nah that has been your albatross [17:39:02] mmmm hmmm [17:39:02] smelling pretty good these days, though! [17:39:37] it's just the maggots that are digging in (to the albotross) [17:39:56] yes they have become beautiful butterflies [17:39:57] I was noticing, in the U.S. Thursday and Friday are holidays this week, right? [17:40:06] yep [17:40:23] and fwiw I'm a jerk and will probably be out Tuesday as usual [17:40:49] ...not a jerk, that's totally cool... [17:40:58] We do want some time to monitor impressions, I think... [17:41:28] right. I'm still hoping for flipping the switch ASAP [17:41:57] definitely will follow your opinion of course [17:42:19] no well I don't have an opinion that's worth giving more weight to than others... [17:43:00] u would be comfortable with a rollout today, and have time? [17:43:06] I had been thinking of trying out and fiddling with more browser tests first, but two days is not long for monitoring impressions [17:43:31] awight, AndyRussG: Start your engines, guys. We can flip the CN switch today. [17:43:37] Oh yeah, this is the highest priority on my plate.. [17:43:45] K4-713: word! [17:43:46] Uhh [17:43:46] ok. woot [17:43:52] party tiiiime [17:43:53] K4-713: has spoken [17:43:57] Bam. [17:44:10] hehehe. AndyRussG was just saying we should check that the ice is clear [17:44:22] * awight inspects more tea leaves [17:44:42] * AndyRussG is still kinda tending towards caution when millions of users are involved... [17:45:04] ...or I like to imagine I am... [17:45:07] we did get burned by that mobile thing... [17:45:54] The best thing I can offer on behalf of an accelerated schedule, is that it's better for users if we deploy further in advance of the big campaign... [17:46:03] Yep [17:46:10] We can still turn on a couple of non-wikipedia projects first [17:46:20] ejegg: Hmmm good point... [17:46:26] the code is deployed everywhere, right? [17:46:34] Yep! deployed everywhere [17:46:35] * awight inspects Special:Version [17:46:36] k [17:47:04] If we had infinite time to play with tests, we might do more w/ browser tests and more variety of allocation cases via the test fixtures [17:47:11] * AndyRussG checks finite watch [17:47:16] that fixture thing... is a serious problem [17:47:31] we need our own instance, and a mechanism for deploying fixtures. [17:47:35] Neither of which exist [17:48:58] Mm well the tests do seem to work [17:49:11] after Zelko's intervention, woot [17:49:12] !! [17:49:18] ah yes thos [17:49:20] those [17:49:32] re:allocations, I was kinda thinking of augmenting the code so it can tests more simultaneous campaigns and various buckets [17:49:39] yes [17:50:26] Also maybe seeing of Krinkle and/or Roan would like to take one last look at our RL stuff before we push the button [17:50:32] s/of/if/ [17:50:52] AndyRussG: well, maybe when K4-713's out of her meeting you two can discuss. [17:51:10] Yeah that was certainly a snafu that Krinkle found, as we were deploying! [17:51:17] shhhh! [17:51:35] Have you been able to test the real-world caching behavior for that RL module, yet? [17:52:07] It appears to work. ejegg witnessed first-hand the version # changing... [17:52:07] like, watch the data contents across the edge of a CN campaign... [17:52:18] ok! [17:52:24] lemme see if I got CN admin rights yet... [17:52:32] right! [17:52:40] AndyRussG: Yeah, I'd love to take a look [17:52:41] We can make that happen fast. [17:52:46] Tell me where and I'll give you a quick check [17:52:47] Krinkle: thanks! [17:53:07] Krinkle: thanks! one sec... [17:53:33] I donno which interface you prefer. https://github.com/wikimedia/mediawiki-extensions-CentralNotice/blob/master/includes/CNBannerChoiceDataResourceLoaderModule.php [17:53:55] Krinkle: the status is it's all now in the CentralNotice master branch and deployed everywhere, but only activated on a few sites via a config variable switch [17:54:07] awight, AndyRussG: I'm not really out of meetings until 1pm or something. [17:54:11] urgh [17:54:19] K4-713: ok we'll fake your signature ;) [17:54:39] It's just squiggles: You'll be fine. [17:55:25] But, on a serious note: Don't rush if you don't think it's a good idea. [17:55:54] AndyRussG: which sites? [17:56:08] or rather, which switch [17:56:10] K4-713: I'm still sniffing and don't see anything tangible to block on. Only common-sense caution. [17:56:12] awight, AndyRussG: You're just clear for whenever, and my feeling is that earlier is much better than later, unless you want to wait until the mid-December slump, or January. [17:56:24] Which is totally valid. [17:56:35] Krinkle: BTW, apologies again for my crazed last-minute mistakes that you commented on in CNBannerChoiceDataResourceLoaderModule::getFromApi()... fortunately that code path (the API bit) is only relevant for 3rd party users... In any case I'll fix a bit later... [17:56:52] wgCentralNoticeChooseBannerOnClient ? [17:56:57] Krinkle: yep! [17:57:13] if ( $lang === 'aa' && $site === 'wikibooks' ) { -> wgDBname === 'aawikibooks' ? [17:57:14] it's enabled on mediawiki.org, aa.wikibooks.org, and test.wikipedia.org [17:57:17] yep [17:57:21] Also on the beta cluster [17:57:28] Krinkle: sorry, that's my rush-hack [17:57:40] It's a bit weird to use lang/site fo that [17:57:46] I've never seen that before [17:58:03] Krinkle: we'll remove that soon, it was the only thing I could guarantee would work in... the 30 seconds I had :) [17:58:08] Reedy might wonder why it's in CommonSettings instead of in a switch in InitialiseSettings, but that's outside my area of concern. [17:58:12] k [17:58:44] K4-713: yeah, earlier makes sense so we can test in a real setting... Do you have a preference w/ regard to what time today? [17:58:52] Nope! [17:59:05] AndyRussG: We're following you guys. Whatever makes sense on your end. [17:59:35] awight: k, got a standup, will get back to you in 30 minutes [17:59:36] Lemme see what's still enabled on Meta [17:59:41] when's your ETA? [17:59:45] AndyRussG: Just please do let us know what makes sense once you figure that part out. :) [18:00:05] Krinkle: awight: K4-713: let's aim for flipping the switch as noon SF time? [18:00:14] today? [18:00:16] AndyRussG: works for me! ejegg ^^ [18:00:17] ye! [18:00:20] For all production? [18:00:24] today [18:00:29] yea [18:00:30] awight: Might need 1-2 hours more if you want my feedback first. [18:00:40] Krinkle: you got it! [18:00:41] I can't move it up that much [18:00:44] OK [18:00:48] any time works for me [18:00:49] Krinkle: no problem, thanks a ton [18:00:54] and sorry for the last minute notice [18:01:00] K 2 pm SF time [18:01:01] ? [18:07:54] AndyRussG: sure. Are you able to work late EST, though? [18:08:05] awight: sure np [18:08:14] ok then [18:08:22] I'll reserve a deployment window [18:08:32] Ah! didn't know we needed one... [18:08:44] I'm not 100% certain, it's just a config change [18:08:55] I'll ask... [18:09:13] (I may have a few interruptions between in the early evening, nothing major tho...) [18:10:58] awight: Order of magnitude? [18:11:04] * K4-713 defects to astronomy [18:12:28] K4-713: wat. I think that's stock jargon [18:12:44] Oh, totally. :) [18:12:55] e.g.: I'd like to order some magnitude, here [18:12:56] dude [18:13:09] I just really like the "Hack off a zero, w/e" [18:13:15] :) [18:13:36] yeah what's a 9x difference, anyway [18:13:41] Pff. [18:13:46] Trifles. [18:13:49] how could that possibly add up to siginificance [18:14:07] * awight makes to edit the WP article on this [18:14:27] AndyRussG: ejegg: ok we're reserved for 14:00-15:00 PST [18:15:04] wohoo! banner boogie [18:15:14] rockin [18:16:33] the-wub: I sent an email, the punchline is that the new contribution_source columns can be used safely, now. [18:17:57] ejegg: AndyRussG: any CR I should be looking at? [18:19:25] awight: not that I know of... I had started very tentatively on the bucket features that would come after this, just to have them on hand in case, but I doubt it make sense to go ahead with any of them before January [18:20:05] awight: one thing for the silverpop export [18:20:07] one sec [18:20:09] AndyRussG: I'm out of the loop on that judgement call... Happy to give feedback though. [18:20:13] ejegg: yep, https://gerrit.wikimedia.org/r/#/c/175163/1/silverpop_export/update_table.sql,unified [18:20:16] looking at it now [18:20:21] cool [18:20:41] wrt the bucket stuff, I think the most urgent for this week would be even more tests, no idea what else is around in other areas... [18:20:43] also I think we should stop deduping on contact ID, at least for opted out addresses [18:21:13] to make sure all opted-out email addresses in the system get exported [18:21:31] and maybe we should add an is_deleted = 0 to the initial select [18:23:22] AndyRussG: sure, I'll work on that! [18:24:00] ejegg: right. oh, that's huge. We're opting out a contact if any of their emails are opted out.? [18:24:17] Yeah, which actually might be wrong [18:24:42] well, definitely is wrong after my editing = add an on-hold address [18:24:59] So we want one row for each address [18:25:48] yep. Lemme see... yeah we should only send to is_primary addresses. All the others should be on the suppression list, so that's simple enuf. [18:28:00] (CR) Awight: [C: 2] "Looks safe" [wikimedia/fundraising/tools] - https://gerrit.wikimedia.org/r/175163 (owner: Ejegg) [18:28:46] (PS1) Ejegg: Export all email addresses for each contact [wikimedia/fundraising/tools] - https://gerrit.wikimedia.org/r/175475 [18:29:22] hi awight! welcome back [18:29:31] (PS1) Ejegg: Do not export email addresses for deleted contacts [wikimedia/fundraising/tools] - https://gerrit.wikimedia.org/r/175479 [18:29:36] atgo-meeting: /me looks around [18:29:48] never mind the PM behind the curtain! [18:34:00] awight: i'm in nyc :P [18:34:27] hey awight do you have any ideas on this one? https://wikimedia.mingle.thoughtworks.com/projects/online_fundraiser/cards/2190 [18:35:20] atgo-meeting: ah, thanks for bumping that one. I need a bit more info. [18:35:27] what do you need? [18:35:30] i've got ccogdill in a call [18:35:35] oh heh [18:36:04] ccogdill: hey, for #2190, you have this postal code data for the AU contacts in a spreadsheet or something? [18:36:30] yep awight, in a spreadsheet [18:36:38] not yet on the server, but it can be :) [18:37:03] ccogdill: ok. I'm not sure whether there's any update/import thing like we would want, give me a few... [18:37:10] okay [18:37:12] thanks! [18:37:25] Usually the built-in import processes insist on a contact ID, afaik. [18:37:33] hmmmm [18:37:34] We can of course custom code something quickly... [18:37:46] okay [18:38:03] well talking to Anne about priorities - there are more important things [18:38:06] so don’t drop everything ? [18:39:12] ccogdill: actually, you might be able to self-serve this one. Please try it on the staging server first. [18:39:24] Go to /civicrm/import/contact [18:39:48] The trick here will be to set "For duplicate contacts" = "Update", and then do something tricky with the Dedupe Rule. [18:40:10] You'll want the dedupe rule to match if the email is a duplicate, with no other fields matching. [18:40:12] okay… let me get there [18:40:37] Also... save the import data as a CSV. [18:41:39] awight I can’t find how to get to “import” from the staging homepage [18:41:49] Contacts -> Import [18:41:50] nevermind [18:41:53] k [18:42:42] so yeah I don’t know what to select for the dedupe rule. am I supposed to create a new one? [18:43:53] ccogdill: you want a rule that only requires a matching email. look at /civicrm/contact/deduperules?reset=1 [18:43:56] I will, too. [18:44:43] ccogdill: all individual contacts, or does this include organizations? [18:45:16] ccogdill: ok I've created a rule "Email is enuf" [18:45:25] ccogdill: try... one or two contacts at a time. [18:45:27] okay awight, thanks! and just individuals [18:45:31] sorry, was wrapping up a meeting [18:45:52] np. you should be set, then. Please ask if anything is unclear. [18:46:58] will do, thanks! [18:47:04] atgo-meeting: that's crazee, I was in Bryant Park just yesterday. enjoy the warm weather, we got blasted by a polar vortex all week. [18:47:16] i was in bryan park on saturday!! [18:47:22] ridiculous. [18:47:38] i bought a hat. because it was cold. [18:47:40] anyway, I see you enough :p [18:47:48] awight actually one more question [18:47:52] ccogdill: yessir! [18:47:57] is there a specific field name I should give the postal codes? [18:48:06] how will Civi know to map to that field? [18:48:14] ccogdill: do these donors already have country=AU, btw? [18:48:19] yep [18:48:22] ccogdill: yeah field mapping is all built into that import wizard. [18:48:35] call it whatever you want, and make the mapping explicit. [18:48:40] okay so field name = postal code will work? [18:48:41] it's on... screen 2 or something. [18:49:03] ccogdill: r u at the mapping screen yet? [18:49:29] ohh… no :D I wanted to have my file right first but perhaps I was getting ahead of myself [18:49:50] ccogdill: yes, call me when you're at the mapping screen, I think it will be self-explanatory-ish [18:49:59] got it [18:50:06] ccogdill: just try a file w/ one or two lines, IMO [18:50:23] k [18:52:38] (CR) Awight: [C: 2] Export all email addresses for each contact [wikimedia/fundraising/tools] - https://gerrit.wikimedia.org/r/175475 (owner: Ejegg) [18:58:44] awight: I'm going to update tools and re-run the table update [18:59:24] ejegg: awesome [18:59:56] (PS1) Ejegg: Merge branch 'master' into deploy [wikimedia/fundraising/tools] (deploy) - https://gerrit.wikimedia.org/r/175487 [19:00:14] (CR) Ejegg: [C: 2] Merge branch 'master' into deploy [wikimedia/fundraising/tools] (deploy) - https://gerrit.wikimedia.org/r/175487 (owner: Ejegg) [19:01:22] !log updated tools from b537e2ec80d16b84f8e0539d4e3d78c8afef1b63 to 113dfe160b750657626e07450003cc88d3939fbd [19:01:26] Logged the message, Master [19:01:57] (PS5) Ssmith: Show actual SQL instead of query string in db modal [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173632 [19:02:20] hey K4-713 we have our weekly during the strategy thing today [19:02:34] Strategy thing? [19:02:39] atgo: please remind me, we aren't trying to get any of the GRabbit reporting changes into Civi until 2015, right? [19:02:45] awight: correct [19:02:46] ejegg pushed that edited patch up for ya [19:02:49] k thx [19:02:54] K4-713: check out yo calendar [19:03:07] atgo: I have no strategy thing. [19:03:18] you're on the invite and accepted it [19:03:23] (CR) Ejegg: [C: 2] Show actual SQL instead of query string in db modal [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/173632 (owner: Ssmith) [19:03:26] it's called "fundraising strategy" and is at 1130 in 61 [19:03:28] Oh, *that* strategy thing. [19:03:29] :p [19:03:37] :P [19:04:07] pizzzacat: good stuff, I'll start on the big english route [19:05:33] cool yay! [19:06:05] so K4-713 i feel pretty good after this morning with megan, unless there's sometehing specific you wanted ot talk about i think we can try to connect tomorrow or wednesday? [19:06:31] atgo: I think I just released the room. [19:06:47] looks like it [19:06:57] Rad. [19:07:13] Ah, I think we covered everything I had for today, so far. [19:07:17] word. [19:07:17] Maybe tomorrow I'll have more. [19:07:26] ...maybe I'll have more in an hour. [19:07:29] yeah there's always time :P [19:08:54] ejegg: I see you're talking with qgil about our Phabricator project--was the decision to use Fundraising, or Wikimedia-Fundraising? [19:10:05] atgo: For what it's worth, I'm on the obt card now. [19:10:22] awight: is that for bug tracking? [19:10:29] i have a fundraising phab project too [19:10:43] atgo: I'm looking at https://phabricator.wikimedia.org/project/view/41/ [19:10:46] rad, thanks K4-713 [19:11:10] oh boy awight [19:11:30] * awight canaries over dead [19:12:17] :P [19:12:36] hey awight - I can confirm my test import on staging went well! the dedupe rule you created isn’t on the production instance of civi though [19:13:21] ccogdill: ok I'll create it there as well [19:13:38] awesome, thank you [19:13:49] ccogdill: great to know this is possible with out-of-the-box stuff! [19:13:54] seriously, so easy! [19:14:41] ccogdill: ssh :) [19:14:56] haha I won’t tell [19:15:24] ccogdill: ok done [19:15:29] merci! [19:16:32] atgo: on this note. What is the pressure like to move our sprint planning into the Facebook tool? [19:16:40] We will survive until next year, I hope? [19:16:46] awight: yes [19:16:51] k.... [19:16:57] we are exempt from the timeline of the other groups :) [19:17:20] awight: sorry, was I actually talking with qgil about Phabricator? [19:17:29] oof [19:17:33] I think I just subscribed to notifications [19:17:59] ejegg: oops, I misread the bizarre threading [19:18:37] 'the Facebook tool', huh? [19:18:40] yeah the threading is one of my complaints [19:18:42] * ejegg senses hostility [19:18:50] argh, now I can parse correctly. The bugzilla stuff will be ported over as a separate project. [19:19:03] :) [19:19:10] hehe. yeah FB was behind the original development of phab, right? Then the guy went crazy and solo? [19:19:27] yeah [19:19:31] Ah, didn't know the origin story [19:19:52] and then the sun disappeared until balance can be restored [19:23:38] atgo: I noticed MG was not on the invite for the strategy thing? [19:23:50] that's true. they are already in the loop afaik [19:23:56] okok [19:24:13] lisa showed me the deck and i was like "man i bet tech would love to see this!" [19:24:22] and she was like "yeah, that's a great idea!" and here we are :) [19:24:40] atgo: that's awesome, thx [19:24:41] I see he thought highly of his fellow PHP FB developers [19:25:10] (the developer of phabricator) [19:25:22] AndyRussG: I bet. but where are you reading? [19:25:48] I remember something in the doc being like "PHP is so easy your dog could program it blindfolded in the dark" [19:26:17] oh heh. cos I mean, regardless of the venality and stark immorality of the whole Facebook paradigm, their devs seem to be doing some amazing things. [19:26:25] considering the language was invented by dogs. [19:26:57] heh [19:28:07] (PS1) Ssmith: Update with numeralJS and fontawesome upgrade [wikimedia/fundraising/dash/src/bower_modules] - https://gerrit.wikimedia.org/r/175493 [19:28:26] ejegg ^ fixed my gerrit issues [19:28:49] which patch did you want me to do the submodule upgrade with? [19:29:15] or does it matter? like, could do it with my next one coming up [19:29:17] pizzzacat: the same one that adds it to bower.json [19:29:30] just so each patch leaves the repo in a working state [19:29:37] oh ok. will see which one that was and then update [19:29:41] rockin [19:29:42] totally [19:40:55] awight: AndyRussG: reviewing now [19:42:36] Krinkle: thanks! [19:42:59] awight: AndyRussG: Hm.. getting warnings in PHP due to use of strtotime() in LocalSettings.php/CentralNotice.php, which is before MediaWIki's date normalisation has been loaded. [19:43:24] basically unable to load any page without hacking mediawiki first [19:44:16] Krinkle: Hmmm... that's unrelated to this change... [19:44:30] Yeah, I'm installing it locally to review the file. [19:44:40] Just noticed my mediawiki suddently didn't work [19:44:52] should probably use a gmt-based date function instead of one inflienced by local timezone [19:48:30] Hmm [19:48:40] OK I'll look into it, not sure why that function was used... [20:06:48] awight: AndyRussG: So the in-class cache, I thought that was removed in a previous revision? [20:07:16] Uh it was... (?) [20:07:16] It doesn't account for resourceloadercontext arguments, and seems obsolete in general. [20:08:26] I wouldn't feel comfortable with that extra caching layer outside the many layers we already have. Afaik there's only ever one call, and if there's more, I fear it has a different context and would pollute other caching layers with wrongly composed data. [20:09:49] Krinkle: K I see, yeah a class variable was added back in because of the call to getChoices() in getModifiedHash() [20:10:09] One is used for the startup module request, the other for the composition request to load.php with the extra module [20:10:21] it'd never be utilitsed for caching [20:11:20] Hmmm... so the'd never be both called in the same Web request? [20:11:31] getModifiedHash is called by modules=startup, getScript is called by the request to load.php?modules=bannerChoiceData [20:11:32] AndyRussG: Nope [20:11:33] I'm not sure we'd want to assume that's part of the RL contract [20:11:48] AndyRussG: And if it were, it might be a different context object, so different user/skin/language etc. [20:11:59] And that is very much part of the RL contract [20:12:07] RL has sufficient caching layers. Not a single module does caching like this afaik. [20:12:53] Krinkle: or better, utc instead of gmt (more precise) [20:13:07] it's possible in theory, and if that becomes an issue adding this in would make sense. Or we could put it in memcached to really cache it well between requests. [20:13:18] Hmmm [20:13:31] but for now I'd recommend removing it as it currently is open to cache pollution [20:13:48] and not needed in the current version of resourceloader as they'd never happen in the same request. [20:14:21] We could certainly remove the instance variable and re-fetch the data each for getScript() and getModifiedHash() [20:14:45] Or we could do as before and create a key to be sure that we're not returning the wrong value [20:15:06] The data retrieval does involve a few DB queries [20:15:30] AndyRussG: But it's not actually being cached. It's an instance variable which is re-created each instance/request. [20:15:37] yes [20:16:13] And for each request, only one of those two paths is chosen. That's part of how RL is designed. Account for that is not realistic. [20:16:43] OK... [20:16:58] if you like, you could match what FileModule and WikiModule do for title mtime caching. Which is to make it an assoative array, keyed by $context->getHash() [20:17:16] (PS1) Ejegg: Add backend Big English config and default filter [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/175501 [20:17:31] choices[$context->getHash()] [20:19:46] Krinkle: I recall having some issues getting all the required info out of $context. For example, $context->getUser I think just checks for a request param, which wasn't sent, as far as I could see [20:20:14] We can also just not store any data in the class, retrieve it afresh each time, and add a comment explaining that it's assumed both methods won't be called in the same request [20:20:34] (CR) Ejegg: "Requesting /data/big-english will give you hourly data for Dec 2014 filtered to amount < 5000, but you can get a different month that with" [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/175501 (owner: Ejegg) [20:20:44] AndyRussG: OK, if there's any doubt at all, just drop that layer for now. It's all very theoretical. For all intends and purposes, it's overhead that doesn't pay off in any scenario rightnow. [20:21:11] And remember, it's heavily cached in front of this. [20:21:22] These are essentially 'compute' classes. [20:21:25] Krinkle: Yeah. I think the priority right now is to be sure we're getting the right data [20:21:41] Adding caching here is hard to review due to the many ways the code is called. I wouldn't trust myself to review that in less than a day. [20:21:44] We're about to start the year-end fundraiser and still need to test a bit on smaller campaigns [20:22:36] Hmmm yeah it's a complicated system! I'm sure I'd take a lot longer to work through all of the details... [20:23:30] In any case even without any in-class caching this should be less load on the backend than the current system, WRT DB selects [20:25:02] So for now if there's any doubt, I think we should get rid of it for now [20:25:45] Alrighty [20:26:26] Krinkle: do you see any other issues (outside getFromApi) ? [20:26:52] BTW most of FR is in a meeting, I'm suree awight and others will see the backscroll in a sec... [20:32:44] awight: ejegg: ^ [20:40:46] (CR) Ejegg: [C: -1] "Something's screwy with the where clause - will fix in another PS" [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/175501 (owner: Ejegg) [20:47:14] (PS1) AndyRussG: Clean up getFromApi() in CNBannerChoiceDataResourceLoaderModule [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/175505 [20:52:07] (PS2) Ssmith: Add board stats piece [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/175161 [20:52:59] ejegg: I just went to the patch where I updated the submodules and ran git submodule add. it for some reason appears that only node_modules was updated, even though bower is what I was going for…. [20:53:14] AndyRussG: If you don't mind, could you change the isset(result->value) to result->getValue() !== null or result->isGood() ? [20:53:23] Format::parse returns a Status object [20:53:41] value is an internal member that is always set from the very beginning, its value may be null however if nothing set it. [20:54:08] I'm not sure how the code came to be accessing ->value, but afaik no prexisting code does that and the documentation reflects similar usage (getValue and isGood) [20:54:30] either value !== null or isGood will do. Whichever you pefer. [20:55:40] Krinkle: OK... I made a separate patches with the fixes 4 silly mistakes in that method... looks like null is also not an accurate check, https://doc.wikimedia.org/mediawiki-core/master/php/html/classFormatJson.html#a38722541c9bf55c894b21643b24016d2 [20:55:46]