[00:32:51] <gerrit-wm>	 New patchset: Jdlrobson; "Story 484: Push nearby to stable" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64453
[00:32:51] <gerrit-wm>	 New patchset: Jdlrobson; "Beta: Stop preview stripping image from nearby list view" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64997
[00:32:52] <gerrit-wm>	 New patchset: Jdlrobson; "Nearby: Use new refresh icon asset and turn off animations" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64998
[00:32:52] <gerrit-wm>	 New patchset: Jdlrobson; "Allow nearby to run at top to avoid flash of unstyled content" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64999
[00:32:52] <gerrit-wm>	 New patchset: Jdlrobson; "Nearby back button tweaks" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65000
[00:32:52] <gerrit-wm>	 New patchset: Jdlrobson; "Improve error handling when location lookup fails" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65001
[00:32:53] <gerrit-wm>	 New patchset: Jdlrobson; "Remove "Needs photo" text" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65002
[00:34:23] <gerrit-wm>	 New patchset: Jdlrobson; "Remove "Needs photo" text" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65002
[00:34:23] <gerrit-wm>	 New patchset: Jdlrobson; "Story 484: Push nearby to stable" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64453
[00:34:42] <gerrit-wm>	 New review: Jdlrobson; "Needs a new asset for photo icon" [mediawiki/extensions/MobileFrontend] (master) C: -2;  - https://gerrit.wikimedia.org/r/65002
[01:30:26] <gerrit-wm>	 New patchset: Jdlrobson; "Redirect page diffs to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[01:33:53] <gerrit-wm>	 New patchset: Jdlrobson; "Redirect page diffs to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[01:35:18] <gerrit-wm>	 New review: Jdlrobson; "PHPUnit tests are failing for me. What did I do wrong?!!" [mediawiki/extensions/MobileFrontend] (master) C: -1;  - https://gerrit.wikimedia.org/r/63907
[10:56:48] <wikipedia-github>	 [Commons-iOS] montehurd opened pull request #74: Fixed super annoying bug with disabled login button (master...disableLoginIfNoCredentials)  http://git.io/9l0muw
[11:48:48] <gerrit-wm>	 New patchset: Yurik; "Fix CSS issue with undismissible warnings" [mediawiki/extensions/ZeroRatedMobileAccess] (master) - https://gerrit.wikimedia.org/r/65110
[11:54:50] <yurik>	 anybody awake?
[11:56:34] <yurik>	 anyone alive?
[11:57:33] <YuviPanda>	 yurik: nope
[11:58:05] <yurik>	 thought so. YuviPanda , could you +2 https://gerrit.wikimedia.org/r/#/c/65110/
[11:58:13] <yurik>	 i added one space :)
[11:58:47] <yurik>	 its kinda funny that one space is the cause of a massive problem with zero :(
[11:59:02] <YuviPanda>	 yurik: I can CR+2, but I can't V+2
[11:59:14] <gerrit-wm>	 Change merged: jenkins-bot; [mediawiki/extensions/ZeroRatedMobileAccess] (master) - https://gerrit.wikimedia.org/r/65110
[11:59:17] <yurik>	 YuviPanda, that should be fine, right?
[11:59:25] <YuviPanda>	 apparently
[11:59:26] <YuviPanda>	 :)
[11:59:31] <yurik>	 V+2 will happen automatically
[11:59:38] <YuviPanda>	 yurik: also, I think preference is for space to be at the end of the class name
[11:59:41] <YuviPanda>	 rather than the beginning?
[11:59:42] <YuviPanda>	 ah
[11:59:44] <YuviPanda>	 no beginning makes sense
[11:59:51] <YuviPanda>	 so you don't end up with a lone class with extra space
[12:00:24] <YuviPanda>	 yurik: yup, that got merged :)
[12:01:06] <yurik>	 YuviPanda, thx!!!
[12:01:10] <YuviPanda>	 :)
[16:25:27] <gerrit-wm>	 Change merged: jenkins-bot; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64972
[17:22:36] <gerrit-wm>	 New patchset: Jdlrobson; "Alpha: Lazy load pages using History.js from search and page content" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/62412
[17:22:42] <gerrit-wm>	 New patchset: Jdlrobson; "Don't lazy load sections on the first load" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65134
[17:23:31] <jdlrobson>	 awjr: can you help me fix up https://gerrit.wikimedia.org/r/#/c/63907/ ?
[17:24:01] <awjr>	 i'll take a look in a min jdlrobson, lemme get to a good stop point
[17:24:04] <jdlrobson>	 i'm a bit confused about php exception handling and how exceptions can get thrown in php
[17:29:39] <awjr>	 ok jdlrobson, looking
[17:30:04] <jdlrobson>	 thanks.. was giving me headaches!
[17:31:09] <gerrit-wm>	 New patchset: Jdlrobson; "Add FIXMES to Zero code" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64607
[17:31:09] <gerrit-wm>	 New patchset: Jdlrobson; "Move zero banner to Zero extension" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65135
[17:32:18] <gerrit-wm>	 Change merged: jenkins-bot; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64607
[17:37:38] <awjr>	 jdlrobson: so it appears to be behaving correctly
[17:37:41] <awjr>	 or at least, as coded
[17:38:03] <jdlrobson>	 awjr: so how come no exceptions get thrown in MobileContext ?
[17:38:06] <awjr>	 you're getting the errors because your provider is providing values that trigger exceptions in MFMockRevision
[17:38:20] <awjr>	 MobileContext?
[17:38:48] <awjr>	 i dunno, is something supposed to be throwing exceptions in MobileContext? (i haven't looked through that yet)
[17:40:24] <jdlrobson>	 there are tests for MobileContext that use the same mock revision
[17:40:30] <jdlrobson>	 throw the same exceptions yet pass some how
[17:40:32] <jdlrobson>	 had me totally confused
[17:40:40] <awjr>	 oh ok, lemme look at that
[17:43:47] <gerrit-wm>	 New patchset: Jdlrobson; "Move zero banner to Zero extension" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65135
[17:43:47] <gerrit-wm>	 New patchset: Jdlrobson; "Treat template banner data as an array." [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65138
[17:44:09] <gerrit-wm>	 New review: Jdlrobson; "waiting for zero team to merge" [mediawiki/extensions/MobileFrontend] (master) C: -2;  - https://gerrit.wikimedia.org/r/65135
[17:47:05] <awjr>	 sorry jdlrobson still tracing how this work
[17:47:06] <awjr>	 s
[17:47:26] <jdlrobson>	 no worries awjr i'm keeping myself busy ;-)
[17:47:32] <awjr>	 :)
[17:50:19] <awjr>	 jdlrobson: it's not throwing exceptions because you never create a Revision object with an id over 200
[17:51:12] <awjr>	 the only rev ids in the provider for testDiffRedirections that you create Revision objects for are 'oldid'
[17:51:20] <awjr>	 and none of the oldid values are > 200
[17:51:52] <awjr>	 note that in MobileContext:redirectMobileEnabledPages(), you never create a revision object for values of the 'diff' param, only 'oldid'
[17:52:05] <awjr>	 make sense?
[17:52:30] <jdlrobson>	 aahhhh
[17:52:34] <jdlrobson>	 but … array( array( 'diff' => 208, 'oldid' => 50 ), false, '' ),
[17:52:43] <jdlrobson>	 why doesn't that throw an exception ?
[17:52:57] <awjr>	 because oldid < 200
[17:53:15] <awjr>	 a Revision object *never* gets created for values of the 'diff' param
[17:53:18] <awjr>	 only 'oldid'
[17:53:32] <jdlrobson>	 ahh i see!
[17:53:33] <awjr>	 jdlrobson: change that oldid value to 250
[17:53:37] <jdlrobson>	 well that's wrong ;-)
[17:53:37] <awjr>	 and it will throw an excepiton
[17:53:40] <awjr>	 :)
[17:54:35] <jdlrobson>	 ookk now we're getting somewhere
[17:54:49] <jdlrobson>	 wait.. awjr array( array( 'diff' => 50, 'oldid' => 208 ), false, '' ),
[17:55:15] <jdlrobson>	 that oldid is over 200
[17:55:47] <jdlrobson>	 yeh and if i DO check for diff it still doesn't throw an exception..
[17:55:49] <awjr>	 oh, yeah actually you have two oldid's in there > 200
[17:55:57] <jdlrobson>	 this puzzles me immensely
[17:56:03] <awjr>	 didnt notice at first - lemme dig some more
[17:56:11] <jdlrobson>	 (but i have made that change to check rev2 does exist
[17:56:43] <jdlrobson>	 actually that said it doesn't need to check it
[17:57:19] <awjr>	 oh jdlrobson because it's not mobile
[17:57:23] <awjr>	 $isMobile == false
[17:57:28] <jdlrobson>	 oohhhh
[17:57:46] <awjr>	 yeah, change false to true and it errors
[17:57:59] <awjr>	 or, excepts, really
[17:59:43] <jdlrobson>	 awjr: perfect.
[17:59:49] <jdlrobson>	 ok so i guess i don't want to throw an error..
[18:00:07] <awjr>	 why throw the exception anyway?
[18:00:18] <jdlrobson>	 awjr: i don't know Max did it :)
[18:00:22] <jdlrobson>	 i'll see how core does it...
[18:00:23] <awjr>	 to emulate looking for non-existent revisions?
[18:00:30] <jdlrobson>	 awjr: yeh that was the idea
[18:00:35] <awjr>	 you can always catch the exception
[18:00:38] <jdlrobson>	 but i don't think MediaWiki throws an exception
[18:00:45] <jdlrobson>	 as otherwise diff pages would not work with bad revisions
[18:00:46] <awjr>	 in fact, you might want to do that anway in MObileContext
[18:00:54] <awjr>	 but yeah, see what core does
[18:00:55] <jdlrobson>	 awjr: also just realized we have no monday next week
[18:01:01] <awjr>	 that is true
[18:01:09] <jdlrobson>	 meaning code review has to happen fast if we want nearby in for tuesday!
[18:01:16] <awjr>	 that is also true
[18:01:25] <awjr>	 what is the current status of nearby stuff?
[18:01:32] <gerrit-wm>	 Change merged: JGonera; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64820
[18:02:00] <awjr>	 jdlrobson: also, it doesnt' appear howie ever updated the card
[18:02:05] <awjr>	 (https://mingle.corp.wikimedia.org/projects/mobile/cards/484)
[18:02:26] <jdlrobson>	 eek awjr i'll hunt him down today
[18:02:42] <jdlrobson>	 awjr: so i'm just waiting on a new asset from may for the upload cta image
[18:02:50] <jdlrobson>	 and for an error page graphic from jared
[18:02:59] <jdlrobson>	 it is my mission to wrap it all up today
[18:03:19] <awjr>	 ok; so the code should be ready (pending review), plus we need to update assets?
[18:08:32] <awjr>	 jdlrobson: ^?
[18:08:44] <jdlrobson>	 awjr: any code that is not -2ed can be merged safely
[18:09:07] <jdlrobson>	 the -2s are minor fix ups that i need to resolve today
[18:09:53] <awjr>	 ok i see, there are a ton of deps
[18:10:06] <awjr>	 but the final couple of patchsets are -2
[18:10:08] <awjr>	 roger.
[18:10:30] <awjr>	 ok jdlrobson, as you're working on fixing up the −2s keep an eye on incoming merges because things will need to be rebased
[18:10:47] <jdlrobson>	 every day i'm rebasing
[18:11:25] <awjr>	 well having a long string of dependent patchsets kidna makes that inevitable
[18:14:30] <gerrit-wm>	 New patchset: Jdlrobson; "Redirect page diffs to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[18:14:31] <jdlrobson>	 awjr: WIN!
[18:14:40] <jdlrobson>	 awjr: yeh i'm use to rebasing ;-)
[18:14:44] <awjr>	 :p
[18:14:54] <jdlrobson>	 ^ but yeh finally got the tests passing
[18:14:57] <jdlrobson>	 ready to go
[18:15:00] <awjr>	 cool i'll take a look shortly
[18:15:02] <jdlrobson>	 YEH TEST DRIVEN DEVELOPMENT!
[18:15:57] <gerrit-wm>	 Change merged: jenkins-bot; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64997
[18:16:59] <awjr>	 i like the new refresh icon
[18:17:04] <awjr>	 i miss the animation tho :(
[18:18:30] <gerrit-wm>	 Change merged: jenkins-bot; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64998
[18:18:53] <jdlrobson>	 awjr: it was fun but a bit silly
[18:19:04] <awjr>	 i am generally pro-silly
[18:19:11] <awjr>	 but, yeah
[18:20:23] <jgonera>	 awjr, are you going to review all the Nearby code? because it's the second commit in a row that you reviewed a minute before I pressed review ;)
[18:20:43] <awjr>	 heh we'll see :p
[18:20:59] <awjr>	 probably not all of it; there's some js heavy stuff that i probably wont feel comfortable +2'ing
[18:22:16] <jgonera>	 are you reviewing this one now: https://gerrit.wikimedia.org/r/#/c/64999/ ?
[18:22:30] <awjr>	 jgonera: looking through it but i wont +2 it
[18:22:34] <jgonera>	 ok
[18:22:53] <awjr>	 same with https://gerrit.wikimedia.org/r/#/c/65000/1 jgonera
[18:23:03] <jgonera>	 ok
[18:23:18] <awjr>	 in fact jgonera, maybe just keep going with nearby  :p i'll start looking at other stuff
[18:23:28] <jgonera>	 no problem ;)
[18:24:14] <awjr>	 jdlrobson: im getting a php strict standards error in your diff code
[18:25:08] <awjr>	 ah because function signature of MFMockRevision::newFromId doesn't match the parent
[18:26:17] <gerrit-wm>	 New patchset: awjrichards; "Redirect page diffs to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[18:27:43] <jdlrobson>	 awjr: eek
[18:27:50] <jdlrobson>	 ^ does that fix it awjr ?
[18:29:29] <awjr>	 yep
[18:33:12] <gerrit-wm>	 New review: JGonera; "Uncaught TypeError: Cannot call method 'on' of undefined nearby-watchstar.js:5" [mediawiki/extensions/MobileFrontend] (master) C: -1;  - https://gerrit.wikimedia.org/r/64999
[18:37:40] <gerrit-wm>	 New patchset: awjrichards; "Redirect page diffs to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[18:39:35] <awjr>	 jdlrobson: i think i found a problem
[18:40:16] <jdlrobson>	 awjr: ?
[18:40:50] <gerrit-wm>	 New review: awjrichards; "(1 comment)" [mediawiki/extensions/MobileFrontend] (master) C: -1;  - https://gerrit.wikimedia.org/r/63907
[18:40:59] <awjr>	 jdlrobson:  ^
[18:41:48] <jdlrobson>	 awjr: super edge case ;-)
[18:41:54] <awjr>	 hardly
[18:41:57] <jdlrobson>	 ummmm it's a good question though
[18:41:59] <awjr>	 articles get created all the time
[18:42:05] <jdlrobson>	 let's see what core does
[18:43:05] <awjr>	 i noticed because i have a newly created article with only 1 rev in my local watchlist
[18:43:59] <jdlrobson>	 awjr: interestedly i noticed we don't show history links on brand new pages..
[18:44:12] <jdlrobson>	 not good
[18:44:18] <awjr>	 on desktop too or just mobile?
[18:44:34] <jdlrobson>	 awjr: just mobile
[18:44:40] <jdlrobson>	 so the desktop way is like so http://localhost/w/index.php?title=New_page&oldid=2611
[18:45:01] <jdlrobson>	 awjr: i'll add a test case
[18:45:09] <awjr>	 :)
[18:45:37] <jdlrobson>	 awjr: can you add a bug for the history link not showing on new pages? we should fix that asap
[18:46:32] <awjr>	 kk
[18:49:00] <gerrit-wm>	 New patchset: Jdlrobson; "(Story 486) Add new editor" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64466
[18:49:29] <gerrit-wm>	 New patchset: Jdlrobson; "(Story 486) Add new editor" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64466
[18:49:45] <gerrit-wm>	 New review: Jdlrobson; "Uncaught TypeError: Cannot read property 'pages' of undefined  when editing main page with no sections" [mediawiki/extensions/MobileFrontend] (master) C: -1;  - https://gerrit.wikimedia.org/r/64466
[18:51:47] <awjr>	 jdlrobson: are you seeing the last modified/history link on master right now on other articles?
[18:52:04] * jdlrobson  looks

[18:52:23] <jdlrobson>	 actually no awjr .. hah
[18:52:24] <awjr>	 i am not :-/
[18:52:32] <awjr>	 heh
[18:52:33] <jdlrobson>	 it's gone walkies in the template cleaning up
[18:52:38] <jdlrobson>	 i'll take a look in a minute
[18:52:42] <jdlrobson>	 probably mega straight forward
[18:52:48] <awjr>	 k im going to wait and hold off on filing that bug
[18:52:55] <jdlrobson>	 sweet
[18:53:02] <jdlrobson>	 i'm just fixing newly created page revisions
[19:01:07] <gerrit-wm>	 New patchset: Jdlrobson; "Redirect page diffs to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[19:01:09] <jdlrobson>	 awjr: so that fixes new diffs ^
[19:01:14] * awjr  looks

[19:02:43] <awjr>	 jdlrobson: you may punch me in the face but
[19:03:11] <awjr>	 how would you feel about moving redirectMobileEnabledPages (or at least the diff-specific logic) into a static method in Special:MobileDiff?
[19:03:22] <awjr>	 i dont think it really belongs in MobileContext
[19:03:23] <awjr>	 (im happy to take a stab at it)
[19:04:00] <jdlrobson>	 arrgghhhhhh
[19:04:03] * jdlrobson  punches awjr in the face

[19:04:05] <jdlrobson>	 ;-)
[19:04:07] <awjr>	 hehehe
[19:04:43] <jdlrobson>	 um not sure how that would work but take a stab at it - would be nice to get it merged soon though
[19:04:48] <jdlrobson>	 can't wait to kill the need for a history page
[19:04:53] <awjr>	 yes
[19:04:54] <jdlrobson>	 also found the problem with last modified timestamp
[19:04:59] <jdlrobson>	 gonna self merge it's a simple one
[19:05:48] <awjr>	 alternatively jdlrobson i can merge your changeset and just move things around alter
[19:05:50] <awjr>	 *later
[19:05:57] <gerrit-wm>	 New patchset: Jdlrobson; "Return the history link" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65148
[19:06:05] <jdlrobson>	 up to you awjr - i just don't want it sitting there too long
[19:06:09] <awjr>	 but yeah the more i think about, the more i dont think that logic belongs in MobileContext
[19:06:12] <jdlrobson>	 ^ awjr history link lolz
[19:06:29] <gerrit-wm>	 Change merged: Jdlrobson; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65148
[19:06:58] <awjr>	 aww jdlrobson that latest MobileDiff changeset still doesn't work for 1 rev artiles
[19:07:08] <jdlrobson>	 awjr: why not?
[19:07:14] <awjr>	 dunno, havent looked yet
[19:07:17] <jdlrobson>	 works for me awjr ..
[19:07:22] <awjr>	 o_O
[19:07:29] <awjr>	 muttley!
[19:09:08] <awjr>	 jdlrobson: because of line 53 in https://gerrit.wikimedia.org/r/#/c/63907/15/includes/specials/SpecialMobileDiff.php
[19:09:18] <awjr>	 if there's only one revision, that will set $prev === null
[19:09:22] <gerrit-wm>	 New patchset: Jdlrobson; "Allow nearby to run at top to avoid flash of unstyled content" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64999
[19:09:26] * jdlrobson  looks

[19:11:22] <jdlrobson>	 awjr: just fixing that up before lunch
[19:16:26] <wikipedia-github>	 [Commons-iOS] brion pushed 1 new commit to master: http://git.io/5ntxXw
[19:16:26] <wikipedia-github>	 Commons-iOS/master 9592d93 Brion Vibber: Fix for enabling/disabling login button...
[19:43:03] <gerrit-wm>	 New patchset: Jdlrobson; "Redirect page diffs to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[19:43:22] <jdlrobson>	 ^ awjr i tried rewriting to move code out of MobileContext but couldn't find a way to do so easily
[19:43:25] <jdlrobson>	 feel free
[19:43:30] <jdlrobson>	 but that covers new pages now
[19:52:20] <awjr_lunch>	 ok jdlrobson, i'll take a look after i eat
[20:29:25] <awjr>	 jdlrobson: i just noticed some HTML markup in a diff of a recent edit i made locally
[20:29:34] <awjr>	 the diff includes two <br> tags
[20:29:46] <awjr>	 (nb i edited the article via mobile)
[20:30:36] <jdlrobson>	 mm that's odd - can you make a qunit test case or send me the source of the html for that page?
[20:30:59] <awjr>	 not sure how to do the qunit test so i'll send you html - do you want html of the page itself, or the diff, or both?
[20:31:08] <jdlrobson>	 the source please
[20:31:35] <awjr>	 jdlrobson: https://gist.github.com/anonymous/16d2823e97724d1748c7
[20:31:49] <jdlrobson>	 great thanks i'll take a look later
[20:35:56] <gerrit-wm>	 New patchset: awjrichards; "Redirect page diffs to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[20:36:32] <gerrit-wm>	 New review: awjrichards; "I'd like to see the diff-specific functionality in MobileContext get moved into Special:MobileDiff, ..." [mediawiki/extensions/MobileFrontend] (master); V: 2 C: 2;  - https://gerrit.wikimedia.org/r/63907
[20:39:17] <awjr>	 huh jenkins didn't merge ^ because when PHPUnit ran it didn't find MFMockRevision; though it works fine for me locally...
[20:47:04] <awjr>	 of course most folks who i think could help are in AMS
[20:47:05] <awjr>	 hmm
[20:51:04] <yurik>	 dfoy, here?
[20:51:45] <Lcawte>	 awjr: Not there yourself?
[20:51:50] <awjr>	 nope Lcawte :(
[20:51:55] <awjr>	 most of the mobile team is not attending
[20:52:12] <awjr>	 i wish, though :p
[20:52:31] <gerrit-wm>	 New patchset: awjrichards; "Redirect page diffs to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[20:53:40] <awjr>	 im afraid to manually merge that patchset too since it will probably cause build failures for all subsequent patchsets
[21:12:05] <gerrit-wm>	 New patchset: Jdlrobson; "Hide Chrome when Wikipedia bookmarked" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65157
[21:18:32] <gerrit-wm>	 Change merged: JGonera; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64999
[21:24:33] <gerrit-wm>	 Change merged: Jdlrobson; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65157
[21:41:57] <dfoy>	 yurik: Found a bug in the "Read in another language" feature - the warning always appears even in the language selected is whitelisted
[21:52:39] <dfoy>	 yurik: I think I may be seeing side effects of modifying X-CS
[21:57:01] <gerrit-wm>	 New patchset: JGonera; "Nearby back button tweaks" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65000
[21:57:24] <gerrit-wm>	 Change merged: JGonera; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65000
[21:58:03] <gerrit-wm>	 New patchset: JGonera; "Improve error handling when location lookup fails" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65001
[21:59:11] <gerrit-wm>	 New review: JGonera; "Needs new copy text, then good to go." [mediawiki/extensions/MobileFrontend] (master) C: -2;  - https://gerrit.wikimedia.org/r/65001
[21:59:23] <gerrit-wm>	 New patchset: JGonera; "Remove "Needs photo" text" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65002
[22:01:30] <gerrit-wm>	 New review: JGonera; "Good to go if you replace the asset." [mediawiki/extensions/MobileFrontend] (master) C: 1;  - https://gerrit.wikimedia.org/r/65002
[22:03:13] <gerrit-wm>	 New review: Jdlrobson; "recheck" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[22:04:11] <awjr>	 jdlrobson: there's a problem with jenkins running the unit tests on ^
[22:04:23] <jdlrobson>	 BAD JENKINS
[22:04:40] <awjr>	 im working on modfying your changes to simplify them and move the diff redirect logic to Special:MobileDiff anyway, which i think will also solve the problem
[22:04:52] <jdlrobson>	 ok :(
[22:06:47] <awjr>	 oh, actually my changes won't solve the problem
[22:06:47] <jdlrobson>	 jgonera: http://pastebin.com/WWVJAgzN
[22:07:47] <awjr>	 anyway, jdlrobson i sent an email to wikitech about the weird jenkins failure
[22:08:03] <awjr>	 since i think anyone who would typically be able to help is on their way to amsterdam atm
[22:08:24] <jgonera>	 jdlrobson, thanks
[22:09:00] <jdlrobson>	 :/
[22:09:16] <gerrit-wm>	 New patchset: Jdlrobson; "Remove "Needs photo" text" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65002
[22:09:51] <gerrit-wm>	 Change merged: Jdlrobson; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65002
[22:11:01] <jdlrobson>	 mm jenkins is +1ing now awjr
[22:11:08] <gerrit-wm>	 Change merged: Jdlrobson; [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/63907
[22:11:14] <awjr>	 oh shit jdlrobson
[22:11:20] <awjr>	 we need to revert that
[22:11:25] <awjr>	 that should not have been merged
[22:11:47] <jdlrobson>	 ? you +2ed it and jenkins verified it..
[22:12:00] <awjr>	 oh; you didn't manually merge:
[22:12:01] <awjr>	 ?
[22:12:13] <awjr>	 yes, you manually merged
[22:12:42] <awjr>	 sorry i should've −2'd it also; i didnt want that manually merged because now we'll likely have the same phpunit test problems on subsequent patchset submissions
[22:13:17] <jdlrobson>	 i hit submit does that not run jenkins?
[22:13:21] <awjr>	 no
[22:13:34] * jdlrobson  confused by jenkins/gerrit once again

[22:13:43] <jdlrobson>	 so what's the damage having merged this?
[22:13:43] <awjr>	 it's a way to bypass the automated tests that run
[22:14:07] <awjr>	 the damage is that the same failure that was preventing automatic merge of that patchset will be present in subsequent patchsets we try to submit
[22:14:19] <awjr>	 actually im not 100% sure of that, but it is likely
[22:14:26] <jdlrobson>	 mm anyway we can test?
[22:14:39] <awjr>	 probably rebase another patchset and try to merge it
[22:14:44] <jdlrobson>	 this one for instance: https://gerrit.wikimedia.org/r/#/c/65138/
[22:14:52] <gerrit-wm>	 New patchset: Jdlrobson; "Treat template banner data as an array." [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65138
[22:14:53] <awjr>	 or, just try to submit it - it should rebase automatically
[22:15:33] <jdlrobson>	 ^ that's a simple patch set do you want to give it a quick review and attempt to merge it?
[22:15:39] <awjr>	 looking
[22:15:45] <jdlrobson>	 thanks
[22:16:06] <awjr>	 right now we're only showing banners to beta/alpha right?
[22:16:34] <awjr>	 what im really asking is, is this going to result in HTML changes to stable
[22:16:38] <jdlrobson>	 no
[22:16:40] <jdlrobson>	 the same html
[22:16:45] <jdlrobson>	 just turns data from a string into an array
[22:16:49] <jdlrobson>	 as it makes more sense here
[22:17:27] <jdlrobson>	 only when notice / zero are set or $wgMFEnableSiteNotice is enabled do banners show
[22:17:39] <jdlrobson>	 i'm still not sure what notice is
[22:17:47] <jdlrobson>	 but it's been there since the dawn of time - i figure zero uses it
[22:18:02] <awjr>	 i think it was for a notificationd isplayed prior to and/or during the sopa blackout
[22:19:30] <gerrit-wm>	 New review: awjrichards; "(1 comment)" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65138
[22:19:34] <awjr>	 jdlrobson: ^
[22:20:14] <jdlrobson>	 awjr: that code need to move out of here any way into zero
[22:20:24] <jdlrobson>	 there's another patch set - could redo that then so wouldn't block on it
[22:20:40] <jdlrobson>	 but yeh looks like you are right that that should be possible
[22:22:51] <awjr>	 what's the other patchset?
[22:23:29] <awjr>	 oh nm i see it
[22:23:32] <awjr>	 ok
[22:23:51] <gerrit-wm>	 New review: awjrichards; "(1 comment)" [mediawiki/extensions/MobileFrontend] (master); V: 2 C: 2;  - https://gerrit.wikimedia.org/r/65138
[22:24:29] <awjr>	 yeah jdlrobson, merge failed of that patchset for the same reason i mentioned above.
[22:24:33] <awjr>	 we need to revert the merge you did
[22:24:51] <jdlrobson>	 wait wait
[22:24:55] <jdlrobson>	 let's work this out
[22:24:58] <jdlrobson>	 why would it be failing
[22:25:08] <awjr>	 see for yourself: https://integration.wikimedia.org/ci/job/mwext-MobileFrontend-testextensions-master/904/console
[22:25:16] <awjr>	 jdlrobson: read the email i sent to wikitech-l
[22:25:59] <jdlrobson>	 why would the class not be present?
[22:26:32] <awjr>	 because the file containing MFMockRevision is not getting included for some reaso
[22:26:33] <awjr>	 n
[22:26:58] <jdlrobson>	 and why might that be?
[22:27:04] <jdlrobson>	 could it be directory structure?
[22:27:12] <awjr>	 i do not know considering it works fine for both of us locally
[22:27:40] <jdlrobson>	 could we add a require in in the mean time ?
[22:28:14] <awjr>	 i suspect it's something in how unit tests are configured to run on jenkins and figured it would be a better use of time to get someone familiar with jenkins and unit tests to take a look
[22:28:31] <awjr>	 we could, but that's a silly hack that we would need to undo later; can't we just wait?
[22:28:45] <jdlrobson>	 awjr: well a silly hack seems better than reverting an entire commit
[22:28:50] <jdlrobson>	 that seems even sillier
[22:29:27] <jdlrobson>	 the weird thing is it doesn't seem to effect SpecialMobileDiffTest
[22:29:53] <awjr>	 that is weird, it could have to do with the addition of the mocks/ dir
[22:30:02] <awjr>	 and something firing out of order; i don't know
[22:30:44] <gerrit-wm>	 New patchset: Jdlrobson; "phpunit test" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65160
[22:30:45] <jdlrobson>	 could it be because it is protected?
[22:30:45] <awjr>	 i don't really see what's wrong with reverting a change that should not have been merged
[22:30:57] <awjr>	 because what is protected?
[22:31:10] <jdlrobson>	 so if i do no score and +2 it won't merge right?
[22:31:17] <jdlrobson>	 (no score for verified)
[22:31:22] <jdlrobson>	 if i hit publish comments
[22:31:59] <awjr>	 so +2 and +2 followed by publish comments will trigger jenkins to rebase and run integration tests
[22:32:11] <jdlrobson>	 ok i'm gonna try that on this patch
[22:32:18] <gerrit-wm>	 New review: Jdlrobson; "does jenkins complain here?" [mediawiki/extensions/MobileFrontend] (master); V: 2 C: 2;  - https://gerrit.wikimedia.org/r/65160
[22:32:18] <awjr>	 which patch?
[22:32:50] <awjr>	 the visibility of that method is not the problem
[22:33:42] <awjr>	 jdlrobson: i really dont think it is worth our time unravelling this; im not positive but i am pretty sure it's something on the jenkins side of things
[22:35:14] <awjr>	 but… we do need to do something about the patchset you merged
[22:35:36] <awjr>	 feel free to try doing a require_once()
[22:35:59] <awjr>	 if you're not happy with reverting
[22:36:07] <jgonera>	 awjr, is there a way of getting the number of page's sections?
[22:36:18] <awjr>	 from the api, jgonera?
[22:36:25] <jgonera>	 not in a hacky way by counting HTML elements or something, because I just discovered it's not very reliable
[22:36:40] <jgonera>	 awjr, preferably not, I'd rather not make a separate API request just to get one number
[22:36:44] <awjr>	 ah
[22:36:48] <jgonera>	 we could set a JS var in PHP
[22:36:54] <awjr>	 hmm
[22:37:00] <awjr>	 not that i know of offhand
[22:37:03] <jgonera>	 but I don't know where to get this from ;)
[22:37:06] <jgonera>	 hm
[22:37:13] <awjr>	 lemme poke around for a sec
[22:37:17] <jgonera>	 ok
[22:37:26] <jgonera>	 I'l ask on #wikimedia-dev too
[22:37:42] <jdlrobson>	 1s meeting
[22:37:52] <awjr>	 oh wow it's 330 already
[22:39:03] <awjr>	 jdlrobson: im alone in the hangout
[22:39:22] <jdlrobson>	 awjr: yeh we're setting up
[22:39:26] <awjr>	 kk
[22:39:32] <jdlrobson>	 howie was on his personal mail and had permission errors
[22:42:13] <awjr>	 jgonera: there's https://doc.wikimedia.org/mediawiki-core/master/php/html/classParser.html#aeb8b16a9e66ef80d30fe78679fbb85db
[22:43:34] <awjr>	 but im not sure that's quite right
[22:43:42] <jgonera>	 awjr, this might be costly I guess and not something we'd want to do on every page view, right? ;)
[22:43:47] <awjr>	 yeah
[22:43:59] <jgonera>	 ok, I'll try to find something
[22:49:19] <awjr>	 wiki content is just so… unstructured, it often seems difficult to do programmatic things
[23:17:30] <gerrit-wm>	 New patchset: awjrichards; "Fix inclusion of MFMockRevision in PHPUnit tests" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65161
[23:18:14] <awjr>	 jdlrobson: ^
[23:18:23] <awjr>	 i think that will fix the problem
[23:25:16] <gerrit-wm>	 New review: Jdlrobson; "win?" [mediawiki/extensions/MobileFrontend] (master); V: 2 C: 2;  - https://gerrit.wikimedia.org/r/65161
[23:25:43] <gerrit-wm>	 New review: Jdlrobson; "no bueno :(" [mediawiki/extensions/MobileFrontend] (master) C: -1;  - https://gerrit.wikimedia.org/r/65161
[23:29:39] <gerrit-wm>	 Change abandoned: Jdlrobson; "test failed" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65160
[23:31:03] <awjr>	 argh
[23:31:59] <awjr>	 so jdlrobson i did discover that you can't really add required classes that do not contain tests in efExtMobileFrontendUnitTests() (or, UnitTestList handlers)
[23:32:39] <awjr>	 because UnitTestsList calls PHPUnit's addTestFile() method which will only add actual PHPUnit tests, not arbitrary classes
[23:32:52] <awjr>	 i am surprised the $wgAutoloadClasses thing didn't work...
[23:34:53] <awjr>	 jdlrobson: so, the changes i'm working on simplify the test cases a great deal, making MFMockRevision needed only in one place. we could copy the class into the same file, which should make the problem go away
[23:41:43] <gerrit-wm>	 New patchset: awjrichards; "Move desktop->mobile diff redirect logic from to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65163
[23:41:54] <awjr>	 jdlrobson: ^
[23:42:09] <gerrit-wm>	 New patchset: awjrichards; "Move desktop->mobile diff redirect logic to Special:MobileDiff" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/65163
[23:42:20] <awjr>	 oops forgot to do that in a local branch :-/
[23:50:30] <gerrit-wm>	 New patchset: Yurik; "Consolidated config, fixed bug in addWarning()" [mediawiki/extensions/ZeroRatedMobileAccess] (master) - https://gerrit.wikimedia.org/r/65164
[23:55:22] <gerrit-wm>	 New patchset: JGonera; "(Story 486) Add new editor" [mediawiki/extensions/MobileFrontend] (master) - https://gerrit.wikimedia.org/r/64466