[18:53:48] <EddieGP>	 Hey, which time will Code-Review Hours be? Documentation at mediawiki says 13:00 in Pacific time, the topic here says 20:00 UTC which would be 12:00 Pacific time (UTC-8 currently). Phab calendar seems to use UTC as fix point. So which one is correct?
[18:55:14] <paladox>	 twentyafterfour ^^
[19:20:43] <andre__>	 MediaWiki.org should be corrected to UTC in that case...
[19:20:47] <andre__>	 I'd guess.
[19:23:05] <EddieGP>	 I would think the same. Especially phabricator seems to always use UTC for it's events, so sticking to Pacific would make things with the times of the events weird.
[19:31:28] <EddieGP>	 Oh, I was wrong about that. Phab is even worse, it sticks to the time zone the creator had in his preferences but does not show any time zone.
[20:28:27] <twentyafterfour>	 :(
[20:28:35] <twentyafterfour>	 EddieGP: that sounds like a bug
[20:28:42] <twentyafterfour>	 in phab
[20:30:17] <EddieGP>	 twentyafterfour: Well I guess it is. I'm just testing at phab-01.wikimedia.org
[20:32:42] <EddieGP>	 The handling of daylight saving is at least confusing.
[20:50:41] <twentyafterfour>	 handling of daylight saving is always confusing
[20:50:53] <twentyafterfour>	 not much of a time saver, more of a time waster
[20:51:12] <paladox>	 lol
[20:51:40] <paladox>	 twentyafterfour you gain 1 hour extra of sleep lol but then again you can stay awake 1 hour extra, so a benefit.
[20:51:54] <paladox>	 Not so much of a benefit going back into utc time.
[21:06:53] <EddieGP>	 Well if we could just all over the world use UTC that would be great. I don't care if the clock shows 8 AM or 11 PM when the sun rises, the benefit of not needing to think about which time to stick and how many hours difference there are between two citys would just outstand this. Not to speek of less programming work. But I think I'm pretty alone with that opinion. ;)
[21:12:40] <twentyafterfour>	 EddieGP: I actually agree with you completely.  Morning is morning, doesn't matter what the clock says
[21:12:58] <EddieGP>	 FYI I filed the problem at T158910
[21:12:59] <stashbot>	 T158910: Time zones on phabricator events are crazy - https://phabricator.wikimedia.org/T158910
[21:13:20] <twentyafterfour>	 Thanks!
[21:13:30] <twentyafterfour>	 So did you have something for code review? I'd be glad to take a look
[21:13:53] <EddieGP>	 Yeah, it's at E514
[21:14:29] <EddieGP>	 Are 5 thinks to be precise, but if somebody would just look in some of them this would be great. Pick how much and what you want ;)
[21:14:54] <EddieGP>	 https://phabricator.wikimedia.org/E514
[21:15:16] <paladox>	 (Off topic) But then if it was miday in the us, it would be the middle of the night in europe
[21:15:21] <paladox>	 would be so confussing.
[21:16:11] <twentyafterfour>	 it's always gonna be middle of the night in europe when it's mid day in the US. Can't change that unless you figure out how to speed up continental drift or build a time machine
[21:17:40] <EddieGP>	 Somebody should teach stashbot also to reply to events, pastes etc., not only tasks. :D
[21:18:18] <EddieGP>	 Just to stop daylight saving would be great start. It doesn't save any energy but confuses almost everybody. Don't know why we still have this.
[21:24:54] <twentyafterfour>	 ok I've +2'd a few of those...
[21:25:20] <EddieGP>	 twentyafterfour: I see, thanks.
[21:25:28] <twentyafterfour>	 not so sure about the last one, changing the ordering could affect performance due to index optimizations and I am not familiar with this code
[21:26:28] <EddieGP>	 We'll I've uploaded that one more for some feedback (see my comment there). I also think it's no really pretty codewise
[21:27:17] <twentyafterfour>	 what's ugly about the code?
[21:27:18] <EddieGP>	 But the parent class does not have support for "order one descending, order other normal", it just can "order all normal" or "order all descending"
[21:27:52] <twentyafterfour>	 well, mysql doesn't use keys when you use ascending, descending order
[21:28:33] <twentyafterfour>	 I guess it probably can't use keys too effectively anyway due to the COUNT()
[21:29:47] <EddieGP>	 I thought about the COUNT maybe being bad for performance. And getOrderFields in my expectation should return a list of fields to order by and not how to order it (DESC).
[21:30:00] <EddieGP>	 That's what I meant with ugly.
[21:30:13] <twentyafterfour>	 ah I see
[21:30:28] <twentyafterfour>	 Well yeah that might be weird but if it works I guess? shrug
[21:31:15] <EddieGP>	 Well as long as nobody changes the parent class it'll work. But I'm not sure if somebody might break it by accident when using the getOrderFields in another way.
[21:38:58] <EddieGP>	 When jenkins says Single space expected before closing parenthesis (MediaWiki.WhiteSpace.SpaceyParenthesis.SingleSpaceBeforeCloseParenthesis)
[21:39:08] <EddieGP>	 does it mean I have too much or too less spaces?
[21:39:21] <EddieGP>	 I can't find any place where those are missing ...
[21:41:27] <EddieGP>	 I have some functions starting their opening parathesis on one line and their arguments starting the next line. Couldn't keep it below 100 symbols per line else (due to indention)
[21:47:30] <paladox>	 You need to add a space for example g) would be g )
[21:52:42] <EddieGP>	 Yeah, I knew that ... typical newbie mistake: When jenkins says look into line 535 you should look into line 525-454 but not only 535 ;)
[21:58:28] <EddieGP>	 twentyafterfour: Could you give https://gerrit.wikimedia.org/r/#/c/338921/ a second run?
[23:26:39] <EddieGP>	 twentyafterfour: I don't know if you want to look into it again, but I've just updated the patch https://gerrit.wikimedia.org/r/#/c/338965 and explained what it does in detail
[23:55:09] <Amir1>	 RainbowSprinkles: twentyafterfour: I want to push this through SWAT but the main patch hasn't been merged. Can you take a look? https://gerrit.wikimedia.org/r/#/c/338701/