[00:10:09] bd808: PSR-3 logging was only backported to 1.23? not 1.24? [00:11:00] tgr: possibly not? [00:12:36] Looks like it. I remember doing it for 1.23 for LTS backports [00:13:22] It shouldn't be hard to port https://gerrit.wikimedia.org/r/#/c/203779/ to 1_24 [00:25:00] bd808: https://gerrit.wikimedia.org/r/#/c/237549/ wanna review? [00:25:32] tgr: should the "@since 1.23.10" lines change? [00:25:56] I left the PSR-3 lib in, it's also included in composer.json but I think 1.24 did not have a strong dependency on Composer yet [00:26:37] well, technically they are since 1.23.10 [00:26:46] but I can change them if that's misleading [00:30:50] hmm... yeah that composer.json is goofy. It's from https://gerrit.wikimedia.org/r/#/c/119939/ which almost got reverted [00:32:08] I don't think having the files in duplicate does any harm [00:32:36] yeah it should be fine. the first autoloader will win [00:34:13] ::ship it: [00:34:36] fixed the @since [00:37:33] thx [00:38:10] sooo, I have these other twelve backports... :) [00:38:33] do I go to hell for self-merging backports? [00:40:48] if they are just backports, I don't think there is a special circle of hell for that, no [00:41:09] but you can probably get Brad, Kunal or Chad to +2 them too [00:42:29] is this for the instantcommons fix? [00:43:18] yes [00:46:15] there's one set down [00:48:29] I can do the rest in the morning if they are still around. I've got to run to dinner now [02:58:22] ori: if you are still up to donating some review time to sentry, https://gerrit.wikimedia.org/r/#/c/199598/ (and https://gerrit.wikimedia.org/r/#/c/237565/ ) should be ready to merge now [03:22:29] tgr|away: I'm not a huge fan of the gigantic command-lines [04:04:00] [Exception ErrorException] (/srv/mediawiki/php-master/includes/tidy/RaggettInternalHHVM.php:5) PHP Fatal Error: Declaration of MediaWiki\Tidy\RaggettInternalHHVM::cleanWrapped() must be compatible with that of MediaWiki\Tidy\RaggettBase::cleanWrapped() [04:05:53] right... [04:06:16] needs = null [04:06:47] yeah [04:07:30] https://gerrit.wikimedia.org/r/#/c/237569/ [04:07:38] needs testing [04:08:04] I figured jenkins would run unit tests with internal tidy [04:08:49] there's no need for either of those defaults [04:09:33] since it's a protected function and the only callers are in RaggettBase [04:15:00] maybe, but consider PHP builtins that take a status parameter by reference. the nice ones let you omit it. [04:15:43] it's horrible to have to create a temporary variable just to please the function, it's like being forced to care [04:16:11] so I think it's a good aesthetic even if it's an internal API [04:17:00] good aesthetic principle [04:19:40] TimStarling: https://gerrit.wikimedia.org/r/#/c/237558/ , then I'm done being distracted by this :P [04:20:41] the instructions in the README still don't entirely work; you end up with a classpath that includes all the dependencies but not html5depurator itself [04:23:31] -cp "$classpath":target/classes \ [04:23:40] html5depurate itself is in target/classes [04:26:26] doesn't work [04:27:19] https://dpaste.de/sDQA/raw [04:29:31] I mean, it works if i add "/home/ori/.m2/repository/org/wikimedia/html5depurate/1.0-SNAPSHOT/html5depurate-1.0-SNAPSHOT.jar" [04:29:36] but I presume there is a nicer way of doing that [05:34:31] ori: you mean posgresql::db? [05:35:05] http://stackoverflow.com/a/16783253/323407 has a nice short command for it, but the long one is already used in posgresql::user so thought it's not the best place to be creative [05:39:38] tgr: yeah, the postgresql stuff. one approach to consider when the execs get unwieldy is to write a small, concise command-line tool, then have the module provision it and use it [05:39:50] also, don't exec echo, just use notice() [05:41:17] https://github.com/wikimedia/operations-puppet/blob/production/modules/graphite/manifests/web.pp takes that approach, if you want to look at an example [05:42:39] you can't make a conditional notice, as far as I can see [05:44:32] yeah, but this is awful [05:44:56] function doThing() { [05:45:34] erm [05:45:53] i hate hiera [05:46:13] it's basically globals [05:46:51] it's bizarre to have a resource that you explicitly depend on and the sole purpose of which is to tell you that it's not going to do the thing you asked it to do [05:48:05] I find the whole use case of having puppet rules to set up things but then deliberately not using them on prod bizarre, but I don't see a better way to implement it [05:48:34] I guess I could just learn how to write resource definitions in ruby [05:49:22] that way lies madness [05:49:30] can't you make the sentry module take a parameter [05:49:37] which tells it whether or not to manage the actual database [05:49:42] ? [05:49:57] and then don't declare the resource if it's not managed? [05:50:20] I could do that, sure [05:52:11] i think that would be much cleaner. and there exists some precedent for it. the nginx module is designed to manage the nginx service in the case of non-critical nginx instances but to leave the service alone in the case of the SSL terminators, since the thought of having puppet restart those is too unpalatable. [17:22:16] bd808: thanks! [17:24:08] tgr: yw. just waiting for zuul to catch up and I'll do the last set [17:26:06] all queued now I think [19:46:45] legoktm: https://gerrit.wikimedia.org/r/#/c/236996/ [20:56:13] ori: are you aware of https://www.mediawiki.org/wiki/Requests_for_comment/Change_LESS_compilation_library ? [20:56:55] AaronSchulz: ok, it's on my list to fix that "properly" (what we did with Echo) [20:57:25] legoktm: no. Excellent, though. Thanks for pointing it out.