[13:22:47] Krinkle: which do you prefer - an deprecation policy with a section on stable interfaces, or a stable interface policy with a section on deprecation procedures? [14:18:25] duesen_: not sure actually, I think both could work. Stability is the overall theme id say, but I terms of what at we "do" due to it, is only deprecating/removing. The rest is what we don't do. [14:18:53] I'd be fine with implicitly renaming deprecation policy page once done, win out making it part of the proposal [14:19:16] duesen_: I see SlotRecord getContent throwing [14:19:35] Via its closure [14:19:59] Should those not be caught if raw access is desired and/or bypassed somehow? [14:21:24] Krinkle: getContent() can throw, but that only happens if that instance was constructed via SlotRecord::newWithSuppressedContent. [14:21:39] This isn't designed very nicely, it was tacked on as an afterthought [14:21:50] it woudl be betetr to have a subclass that is guaranteed not to throw. [14:53:10] Krinkle, duesen_ since the ie6/ie7 support rfc has been approved, do you have a sense of the timeline for dropping the code ( WebRequest::checkUrlExtension() ) that triggered that rfc? without that, restbase (/cc mobrovac) and maybe flow will have to resort to workarounds for dealing with parsoid rest api requests containing . in the title ... similar to T232556 [14:53:10] T232556: Parsoid/PHP roundtrip testing: Period ( . ) in titles fail with a http 403 (Invalid file extension found in the path info or query string.) error - https://phabricator.wikimedia.org/T232556 [14:54:23] maybe i'll file those tasks for restbase and flow and we can decline them if the core code gets dropped. [14:54:40] duesen_: right, but how do I know the class where I got slot record from didn't use that method [14:54:52] As a way to delay the permission check [14:54:58] Which seems to be its purpose ? [14:55:13] So you can get an array of slots first [14:55:21] And to know which one is surprised [14:55:26] Makes sense [14:55:50] Can you comment on that ask explaining why it's impossible to throw here? I believe you :) [14:56:54] Krinkle: does the doc improvement here answer your question? https://gerrit.wikimedia.org/r/#/c/542101/ [14:57:04] if not, please comment on that patch, so i can further clarify. [14:58:40] is this comment insufficient? https://phabricator.wikimedia.org/T235168#5562834 [16:17:59] duesen_: OK. That makes sense. I still feel less than confident in knowing when it can and can't throw. Is there an example of when it could lazily throw? Eg @throws .. If lazy perm checks fail. Eg when using it via X. [16:18:26] I'm having to assume that rev getSlow doesn't use newSurpressed [16:18:33] Which is true I guess? [16:18:46] RevisionSlots::getSLot() does not. [16:19:01] RevisionRecord::getSlot() does - it takes an $audience parameter [16:19:21] a slot you get from RevisionRecord::getSlot() may throw if the content has been suppressed [16:19:42] one you get from RevisionRecord::getSlots()->getSlot() will not throw. [16:19:55] confusing, i know. getSlots should probabyl be getSlotsRaw or some such [16:20:27] the entire audience check logic is wonky. it ties into the ActingUser stuff as well [16:21:37] duesen_: Yeah, I understand it now, but I feel like the current docs proposed wouldn't have given me that understanding. It explains that some forms of creating SlotRecord can result in SlotRecord::getContent throwing, but not that RevRec::getSlows won't use that. Some re-confirmation like @return SlotRecords[] These will be raw and not throw upon accessing getContent [16:21:48] or some such, I know it's verbose, but in the current setup that might be the verbosity we need? [16:22:06] maybe [16:22:14] can you comment on the patch i made to improve the documentation? [16:23:49] done [16:24:54] ah right, now i see what you mean. [16:24:57] will improve [16:26:07] duesen_: I'm admittedly very unfamiliar with this category of classes, so I might not be the best audience for this. Will let you decide and then +2 [16:26:58] i think you are the perfect audience, because you are unfamiliar with these classes ;) [16:27:59] Right, I see. Kinda worked my way into a paradox. I guesss we generally don't sit down to "learn" something but use this to navigate bit by bit. Works for me :) [16:36:15] Krinkle: updated [22:27:39] Anyone know how to decode these from phpunit? [22:27:40] 23:16:10 1) MediaWiki\Tests\Storage\NameTableStoreTest::testGetAndAcquireId with data set "no wancache, empty table" (EmptyBagOStuff Object (...), true, 1, array(), 'foo', 1) [22:27:40] 23:16:10 Trying to configure method "insert" which cannot be configured because it does not exist, has not been specified, is final, or is static [22:27:40] 23:16:10 [22:27:41] 23:16:10 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424 [22:27:43] 23:16:10 /workspace/src/maintenance/doMaintenance.php:94 [22:28:47] 427 on a different branch... [22:29:01] https://github.com/wikimedia/mediawiki/blob/REL1_33/tests/phpunit/MediaWikiTestCase.php#L427 [22:33:31] ah hang on, more test refactoring...