[07:37:10] New patchset: Henning Snater; "(bug 37460) improved tooltip's mouse button recognition" [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/11017 [09:27:28] JeroenDeDauw: would you have a look at the "untangle" patches in the review queue? the changes are touching mostly code you wrote, I think [09:29:16] once this has been merged, I'll have another look at the "simplification" patches. i guess the one for libs can be merged easily enough [09:29:23] Daniel_WMDE: I am looking at one of your stuffs ATM [09:29:30] oh, cool [09:29:52] This one https://gerrit.wikimedia.org/r/#/c/10997/ [09:30:08] yea. [09:30:19] the two changes have to be reviewed, tested and merged in tandem. [09:30:33] one doesn't work without the other. they are, after all, fixing a cross-dependency :) [09:30:44] (oh this will be so much easier with a single repo) [09:31:16] yeah [09:31:28] moin Jens_WMDE [09:31:31] Daniel_WMDE: we just discussed this during scrum [09:31:36] Will do that tomorrow [09:31:49] So trying to get rid off all of the reviews by then we should [09:31:55] indeed [09:32:13] morning [09:33:50] Daniel_WMDE: I'm getting an error when running the tests [09:33:54] Daniel_WMDE: did they all pass for you? [09:34:25] Oh nvm [09:34:41] Daniel_WMDE: my fault - need to stash some local change [09:34:49] http://tobiaszugehoer.blogsport.eu/files/2011/12/Shall-Not-Pass.jpg [09:36:28] Daniel_WMDE: incorrect indent on l273: https://gerrit.wikimedia.org/r/#/c/10998/1/Wikibase.hooks.php,unified [09:36:30] The horror! [09:38:55] Daniel_WMDE: why did you move CONTENT_MODEL_WIKIBASE_ITEM to the lib? [09:39:08] JeroenDeDauw: because it'S used in the constructor of Item [09:39:38] Meh, bad [09:40:13] Ok, let's fix that together with the rest of the stuff that the client should not know about that's in item now [09:40:35] JeroenDeDauw: this is something i'd like to discuss with you tomorrow in detail... Item really needs to do quite different things on the client and the repo. but it also has shared aspects. I'm not sure yet how to best model this. Subclasses? Delegation? Unrelated classes using some helper methods to avoid coide replication? [09:40:43] yes, exactly [09:42:19] New patchset: Daniel Werner; "Bug solved in Chrome, backspace will now be considered to be able to change a value which will result in certain events being triggered for EditableValue.Interface. Achieved by using keypressed instead of keydown event." [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/11019 [09:43:31] Daniel_WMDE: another options is to move the static stuff in Item to ItemHandler [09:43:44] Daniel_WMDE: but let's do the actual work once this is in one repo :p [09:44:25] New review: Jeroen De Dauw; "(no comment)" [mediawiki/extensions/WikibaseLib] (master); V: 1 C: 2; - https://gerrit.wikimedia.org/r/10997 [09:44:26] Change merged: Jeroen De Dauw; [mediawiki/extensions/WikibaseLib] (master) - https://gerrit.wikimedia.org/r/10997 [09:44:27] New review: Jeroen De Dauw; "(no comment)" [mediawiki/extensions/WikidataRepo] (master); V: 1 C: 2; - https://gerrit.wikimedia.org/r/10998 [09:44:29] Change merged: Jeroen De Dauw; [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/10998 [09:44:56] JeroenDeDauw: yes. there are even more options, like making ContentHandler into a factory for WikiPage, and implementing an ItemPage that handles all the loading and storing [09:45:02] which is indeed what WikiPage is for [09:46:04] Daniel_WMDE: as long as we have sane interfaces and sane separation of concerns in the extensions I'll be happy and not care much about further details [09:47:58] New review: Jeroen De Dauw; "(no comment)" [mediawiki/extensions/WikidataRepo] (master); V: 1 C: 2; - https://gerrit.wikimedia.org/r/11019 [09:48:00] Change merged: Jeroen De Dauw; [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/11019 [09:52:05] I promise I won't tweet it [09:52:05] damn [10:02:12] JeroenDeDauw: would you fix the merge conflict in I66c985ae, so i can review and merge? [10:02:28] if i fix the conflict, we have to find someone else to review & merge :) [10:10:43] New review: Jeroen De Dauw; "(no comment)" [mediawiki/extensions/WikidataRepo] (master); V: 1 C: 2; - https://gerrit.wikimedia.org/r/11017 [10:10:45] Change merged: Jeroen De Dauw; [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/11017 [10:12:32] Daniel_WMDE: I'm not fixing it [10:12:40] TobiasG_WMDE: Don't forget about https://gerrit.wikimedia.org/r/#/c/10685/ [10:21:16] Denny_WMDE: Daniel_WMDE: I think that we also should have a talk about to what extend we want to unify everything into being properties once we actually have those [10:21:27] As I understand it, Daniel_WMDE wants to unify all of the things [10:21:47] But I'm a bit skeptical this will work or be better then leaving some stuff out [10:22:02] Probably good to have this figured out soonish [10:23:49] Oh, I don't think we should unify all in properties, not at all [10:24:05] Label, description, Alias, sitelinks, for starters, all out [10:29:56] * JeroenDeDauw shouts *fight* [10:30:05] The winner gets a cookie? [10:35:54] idea for food: http://falafelday.com/ [10:37:00] New patchset: Jeroen De Dauw; "Add sites table, add local field to langlinks and drop interwiki." [mediawiki/extensions/WikibaseLib] (master) - https://gerrit.wikimedia.org/r/11031 [10:37:37] Jens_WMDE: oh, I should not have panicked [10:37:44] Apparently it did not delete it [10:37:54] oh! [10:38:04] interesting [10:38:46] Jens_WMDE: I forgot that reset --hard does not actually remove commits [10:40:52] oh, it was committed, but not pushed? [10:41:00] ah! [10:41:09] yes, this changes EVERYTHING! [11:01:30] Denny_WMDE: i'll tell you tomorrow why i think that's a very bad idea :) [11:04:03] Daniel_WMDE: OK - I'll make sure to have some extra time for that :) [11:07:14] ok [11:07:16] hmpf [11:07:25] gah, screw procedure. [11:18:49] Jens_WMDE: i'm confused about the merge conflict. according to blame, the offenting line is old (May 24) and comming from your commit back then. [11:18:52] err. [11:18:54] sorry [11:18:58] JeroenDeDauw: --^ [11:19:06] JeroenDeDauw: this line: $success = array_key_exists( $siteId, $this->data['links'] ) && $this->data['links'][$siteId]['title'] === $pageName; [11:19:33] i'm currently trying to figure out how to best solve the conflict. [11:19:51] apparently, it's not possible to commit a merge with --amend [11:20:26] bah, i'll make a new patch [11:21:20] Daniel_WMDE: looks like something I indeed have not touched recently [11:21:43] according to git, no one has. and yet, it reports a merge conflict. [11:21:45] wtf? [11:23:50] New patchset: Daniel Kinzler; "Simplified internal structure and get rid of methods that expose internal structure and modified tests to match" [mediawiki/extensions/WikibaseLib] (master) - https://gerrit.wikimedia.org/r/10728 [11:28:43] New review: Daniel Kinzler; "the new patchset was just needed to get git unstuck on a spurious merge conflict." [mediawiki/extensions/WikibaseLib] (master); V: 1 C: 2; - https://gerrit.wikimedia.org/r/10728 [11:31:36] New patchset: Daniel Kinzler; "Simplified internal structure and get rid of methods that expose internal structure and modified api and tests to match" [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/10727 [11:34:33] JeroenDeDauw: ok, got both changes un-conflicted. but now Wikibase\Test\ApiSetItemTests::testSetItemGetTokenSetData is failing in a very interesting way [11:34:50] strpos() expects parameter 1 to be string, array given [11:34:51] /DATA/var/www/daniel/wikidata/includes/Sanitizer.php:582 [11:35:00] Sanitizer?! why is it even trying to generate html? [11:35:07] i guess i'll have to dig in... [11:35:50] New review: Daniel Kinzler; "test failed, will dig in to find out why:" [mediawiki/extensions/WikidataRepo] (master); V: -1 C: 0; - https://gerrit.wikimedia.org/r/10727 [11:38:41] eeewww.... [11:39:02] ItemView::render prints to the OutputPage *and* returns a ParserOutput? [11:39:04] wtf? [11:39:08] why? [11:43:52] office is mostly on lunch break right now [11:44:02] just in case you expect an answer from JeroenDeDauw [11:47:33] no, i'm just randomly voicing my dismay. [11:47:48] we are supposed to clear out the review queue, right?... [11:47:56] what a shame, there are several things that need fixing badly. [11:53:30] yes, it would be convenient for the reorg tomorrow to have no queued items [11:54:09] o_O [11:54:18] this patch is horribly broken [12:03:36] New patchset: Daniel Kinzler; "Simplified internal structure and get rid of methods that expose internal structure and modified tests to match" [mediawiki/extensions/WikibaseLib] (master) - https://gerrit.wikimedia.org/r/10728 [12:04:03] New patchset: Daniel Kinzler; "Simplified internal structure and get rid of methods that expose internal structure and modified tests to match" [mediawiki/extensions/WikibaseLib] (master) - https://gerrit.wikimedia.org/r/10728 [12:12:58] gah. [12:13:01] still broken [12:13:04] or even more so. [12:13:10] i give up on this one. it's just screwed. [12:18:24] New patchset: Tobias Gritschacher; "replaced window.mw by mw as other extensions doing it the same" [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/10685 [12:19:59] Daniel_WMDE: I +1 your eeeewww. Danwe_WMDE did this, even though I was raging to him the whole time this was bad [12:21:59] JeroenDeDauw: i'm currently fighting with your simplification patch, though. something major appears to be broken now, will try to find that in a minute (need a short break). [12:22:13] but i found something that can't ever have worked. you sure you tested this? [12:22:34] Daniel_WMDE: all the tests passed [12:22:55] then maybe there was no test for this, and now there is. [12:23:27] JeroenDeDauw: http://pastebin.com/mcJU3kSq [12:23:33] I have not seem new tests go throguh gerrit [12:23:44] i have no idea, but that code is wrong [12:24:05] hm... the wrongness only happens when the "compatibility" mode is used, though [12:24:13] which may or may not be the case for some tests [12:24:29] anyway - that bit is fixed now (review/merge pending) [12:24:58] but tests for the other patch are screaming murder. apparently, arrays show up where stringts are expected. fails deep down in the database code [12:25:05] probabloy something trivial... [12:25:11] will try to find out in a minute. [12:25:37] Daniel_WMDE: I ran into such stuff when running the tests initially [12:25:46] But fixed it before submitting my stuff to gerrit [12:26:56] well, somethign unfixed it... [12:27:09] Clearly [12:27:19] Daniel_WMDE: right, wrong key there, good catch [12:27:26] Wonder how the tests did not catch that one [12:27:49] Probably should have a look at coverage after this stuff got fixed [12:35:41] New patchset: Tobias Gritschacher; "JSHint supported code cleanup" [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/10685 [12:40:22] JeroenDeDauw: oh great, i'm getting a mix of old-style and new-style entries in some cases. getMultilangText fails to detect it, because the first item is ok. [12:51:03] New review: Jeroen De Dauw; "(no comment)" [mediawiki/extensions/WikidataRepo] (master); V: 1 C: 2; - https://gerrit.wikimedia.org/r/10685 [12:51:06] Change merged: Jeroen De Dauw; [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/10685 [12:57:40] Daniel_WMDE: Item::cleanStructure is a better place to put this stuff [12:58:00] Daniel_WMDE: you can just ditch your current data ofc [12:58:02] yea, i already fixed this one [12:58:38] now i'm trying to find out why wbgetitems gets "false" as an ID. [12:58:47] there's way to little input validation in our code [12:59:05] passing arrays instead of strings or false for an id should fail very early [13:04:18] TobiasG_WMDE: we should have a giant thinghy above your head so it looks like this: http://4.bp.blogspot.com/-NhE7TRP_RLo/T4-GQZ41WZI/AAAAAAAAGAo/5uWXwI9g49k/s400/cat084.png [13:05:34] JeroenDeDauw: in wbgetitems, what should happen if one of the requested items is not found? should the entire thing fail? [13:05:48] should it just skip the item? [13:06:26] hm, isn't John online? [13:07:53] Daniel_WMDE: I think it should "skip" the item but have an error in the output indicating it was not returned for reason foobar [13:08:00] humpty hump.. you called master? [13:08:03] It's a john [13:08:20] :P [13:10:23] Hm, I think it should return a failure on one of them, but I think it now returns an error [13:10:24] jejebinks: currently, non-existing items would just be skipped silently. [13:10:27] jejebinks: how would you indicate a failure for one of them? [13:10:28] Its been changed so many times that I'm confused.. [13:10:28] well, what *should* it do? [13:10:55] The thing is we transform site-title pairs into ids and it is then not obvious anymore who is failing [13:13:01] the id list is resolved into items [13:13:01] In that case someone ask for a specific item that does not exist and it can be reported [13:13:03] yes... [13:13:04] how? [13:13:15] err.. [13:13:41] without failing the entire request, i mean [13:13:54] and in a way that makes it clear which title wasn't found. [13:14:28] Well, actually, I don't like GetItems.. but let me see.. [13:15:07] Daniel_WMDE: to answer about your "ItemView::render prints to the OutputPage *and* returns a ParserOutput?" [13:15:20] yes it does, ParserOutput should be adjusted in core a bit [13:15:54] Creating a fake entry for the item with the failing id and there add id=-1 [13:15:55] so we don't have to do that. Not sure however about the page title change though, that should probably still be moved somewhere else [13:19:25] I add a bug for it and we'll fix it after the merge, ok? [13:19:58] Danwe_WMDE: actually, the page title change should be in the view implementation. the rest should not. [13:20:13] Danwe_WMDE: everything that should be doen to the ParserOutput should be in the ContentHandler [13:20:30] ParserOutput objects will be constructed even if the item is not currently being shown on the page [13:20:43] mixing the logic for analysis, rendering and display will break things badly [13:21:13] Danwe_WMDE: basically, consider that the item sometimes needs to be rendered without being shown. [13:21:24] to update a cache entry or some such [13:23:00] Daniel_WMDE: Don't think taht's really possible without a ParserOutput. Also, ContentHandler is calling Parser::parse() for wikitext... Also, a ParserOutput can be created without actually showing it. But all stuff done to the OutputPage should only be done when REALLY showing it, for example in the view action. [13:23:32] that'S not a problem [13:23:53] * JeroenDeDauw thinks daniel is right [13:23:57] if you add modules to the parseroutput, nothing happens, unless someone reads this stuff from the parseroutput. [13:24:17] JeroenDeDauw: Kalisti! [13:24:36] Daniel_WMDE: whut? [13:25:01] Danwe_WMDE: ParserOutput is just a stipd holder. it doesn't do anything. adding unnecessaqry stuff to it doesn't hurt. [13:25:17] doing things to OutputPage *does* hurt. that's a singleton. [13:26:02] Daniel_WMDE: it holds information necessary for the actual output [13:26:09] JeroenDeDauw: err, Kallisti. Never mind. [13:26:35] Danwe_WMDE: yes. [13:26:44] Daniel_WMDE: as I said before, OutputPage shouldn't be messed around with there, I agree. But that will require some mods to core [13:27:12] Danwe_WMDE: so, getParserOutput can do whatever it likes with its newly created ParserOutput object, and return it. But it must not screw with the OutputPage [13:28:04] The actual rendering in the view action then uses the ParserOutput created by getParserOutput, and hands it to OutputPage for display [13:28:14] and it may do moire stuff to the outputpage,. like setting the display title [13:28:25] Daniel_WMDE: See ItemView.php line 149, I already put a NOTE there telling you all aof that... [13:28:33] // NOTE: instead of calling $this->getOutput(), at least addJsConfigVars() could be implemented into ParserOutput, [13:28:35] // right now it is only available in the OutputPage class, using this here might be kind of hacky. [13:28:56] Daniel_WMDE: just looked it up - falls nicely into the troll category :) [13:29:02] no, that's not what i mean. that's the wrong way around. [13:29:21] JeroenDeDauw: Devine trolling! [13:29:49] JeroenDeDauw: you didn't read Illuminatus!? you must! [13:30:12] Danwe_WMDE: lets figure that out tomorrow, i'm currently trying to wrestle the api into submission [13:31:07] Daniel_WMDE: file it in bugzilla already [13:31:13] So I don't loose track of it [13:31:31] JeroenDeDauw: Good Idea. [13:31:37] Daniel_WMDE: Alright, tomorrow [13:31:38] go ahead :) [13:35:54] Daniel_WMDE: you do realize that if he says "tomorrow", that actually means 28 "half hours"? Not sure this is before or after the sun goes out - TobiasG_WMDE can I get an expert opinion on that? [13:36:48] JeroenDeDauw: the api is crumbling to pieces, that kind of has my attention right now :) [13:37:28] Added as Bug 37501 [13:42:17] items in apigetitems are modelled after output in query, but that module has strange error reporting when something isn't found [13:42:45] I guess the simplest is the ting done if several pageids are missing [13:45:11] That would be consistent with how our ids are handled, but it will not be consistent with lookup of with use of site-title when those are plural [13:51:31] jejebinks: https://bugzilla.wikimedia.org/show_bug.cgi?id=37504 [13:51:56] Danwe_WMDE, JeroenDeDauw: https://bugzilla.wikimedia.org/show_bug.cgi?id=37502 [13:52:46] Danwe_WMDE: fix it already [13:53:40] ugh, guys... [13:53:44] not more merge conflicts! [13:54:33] jejebinks, JeroenDeDauw: i'm really getting desparate about this spi stuff. seems like setitem doesn't set items. or something [13:54:36] gah. [13:55:53] Well, if Daniel_WMDE ends up exploding, we'll still have another daniel [13:56:19] "Jeroen what should I fix? Oh the bugzilla thing? Oh, I don't have time for that right now" -- Danwe_WMDE [13:56:49] "He created a bug for it?!" -- Danwe_WMDE [13:57:06] "Oh where is the popcorn" -- TobiasG_WMDE [13:57:09] me want popcorn [13:58:00] actually, i'd fix it myself. but first i have to find out why this "simplification" patch isn't working [13:58:10] something is utterly broken... probably just one line :) [13:58:19] but i can tell you one thing [13:58:26] MORE INPUT VALIDATION! [14:02:12] I think you overlooked one thing with apisetitem, its not a final solution, its a quick fix to get thing going. [14:02:41] If you don't like it we can simply scrap it and route such requests to the special page. [14:02:42] Daniel_WMDE: you absolutely sure it's not a bunch of lines that are just a little broken? Might be hard to distinguish from one horribly broken line... [14:02:47] * JeroenDeDauw hides [14:03:28] Only one that really need the apisetitem is the bot-things. [14:03:58] Daniel_WMDE: Added an dependencie to Bug 37501. Still not fully understanding your problem here, still think we're talking the same here. [14:05:41] Daniel_WMDE: Just tell me please whether in your opinion it is right for the ItemView::render() to create a ParserOutput [14:06:41] Danwe_WMDE: it should call another function to create the parser output, and use it. [14:07:12] the other function, let's call it makeParserOutput, must not have side effects. and would be called by the handler [14:09:00] Daniel_WMDE: I see it the other way around, render() should create a ParserOutput (or better a RenderOutput because 'ParserOutput' is just the wrong word), and then there should be another function to prepare its result, the output for an actual OutputPage when it's really going to be displayed. [14:09:18] ItemView::render() basically is the equivalent to Parser::parse() [14:10:40] Danwe_WMDE: ok then, just a question of naming. the logic for displaying it should be in ViewItemAction. [14:10:56] if render() is to behave like parser(), it should not have sideeffects. [14:11:10] but it's no problem, because the stuff you are doing the with OutputPÜage can easily be done in the view action [14:11:47] Danwe_WMDE: sure, should be in ViewItemAction, the one line to adjust the title. [14:12:17] Danwe_WMDE: and also addJsConfigVars [14:12:18] for the other lines Bug 37501 will resolve the uglyness [14:13:19] Danwe_WMDE: i don't understand that bug report - what does that method do? why do you think it should be in parseroutput? [14:15:20] Daniel_WMDE: Basically, in ItemView::getHTML() we already create output specific to one language. By doing the addJsConfigVars() in render() instead of ViewItemAction we make sure the same language will be used. [14:16:06] Danwe_WMDE: the caller controlls the language, no? [14:17:10] true but the output could be handed round. It's also something the rendered output totally requires on so if it isn't added it won't work and that's why the render() should already add it. [14:17:50] We also add the resourceloader modeuls there so why not the JS variables the modules require... [14:17:50] Danwe_WMDE: the rendered output requires some JS? probably also a bad idea... [14:17:56] but again, let's discuss this tomorrow [14:18:00] then use getHTML() [14:18:10] i need to fix the damn api, so we can finally merge these patches, before i go crazy [14:18:42] Danwe_WMDE: getHtml doesn't insert the DataUpdate stuff we need. that's the other very important part of the ParserOutput [14:18:46] anyway. [14:19:06] jejebinks: can you explain ApiSetIte3mTests::linkSiteId to me? [14:19:09] i'm confused. [14:19:47] it inserts some site link into item A, and then checks whether we have an item for the target of that site link? is that right? [14:20:02] for me, this always fails. the item isn't there. where and when would it have been created? [14:20:02] Daniel_WMDE: DataUpdate stuff should perhaps be added by render() ? getHTML() is just if you want HTML representation of an item. render() will add optional JS stuff, it's not really requires but we shouldn't add half of it only. Either full or not at all [14:20:40] Danwe_WMDE: exactly! i need a function that createa a parseroutput object, has no side effects, adds the dataupdate things, and (optionally) the html. [14:20:47] that's how getOutputObject is specified. [14:20:56] err, getParserOutput [14:20:57] sorry [14:22:58] Daniel_WMDE: "(optionally) the html" ? [14:23:18] oh, i see [14:23:20] that must be new? [14:24:09] Danwe_WMDE: yes. that's somewhat silly, but after trying it 5 ways, i ended up using this one. the parseroutput object represents everythign that the framework needs to know about the content, for displaying and saving it. like the secondary data stuff. sometimes, we just need to update the links tables. there's no need for the actual html. [14:24:22] this is actually why the method is called getParserOutput(), and not render() [14:24:36] the last parameter controlls the html generation [14:24:53] for wikitext, analyziong the content and generating wikitext is the same thing, of course [14:25:17] Still have some questions there, but lets discuss it tomorrow after all [14:26:58] testLinkSiteIdAdd is first run and it create items if they are missing and checks if they are okey. [14:28:48] Then testLinkSiteIdUpdate is run on the same items and checks if they are edited [14:29:01] Then the same thing with a set operation [14:29:37] jejebinks: ugh, so the test cases depend on each other, and on the order they are executed in?... [14:29:53] Error reports can be weird du to accumulated items in the temporary base [14:29:54] not good... and doesn't seem to be working for me. [14:30:03] i'll have to look more closely though. [14:30:07] aude: didn't really read about it yet but http://pdfapplied.challengepost.com/submissions/8241-openstreetmap-change-tracker [14:30:12] Old tests does not handle that correctly and gives errors [14:30:35] jejebinks: i merged current master [14:30:40] Daniel_WMDE: Yes, but only inside the file [14:30:53] jejebinks: yes, but i'm not sure there are any guarantees even there. [14:31:09] for one thing, you can use --filter to only execute specific test cases [14:31:42] jeremyb: hey :) i just tried wikidata.org again and it still redirects me to wikipedia - is that a dns change delay or is it not changed yet? [14:32:23] yeah I know you can, but with a temporary database that accumulates entries it would be kind of difficult to avoid dependency between the tests.. and note that the tests are marked as dependent on each other [14:33:20] ah, if the dependency is declared, that's much better [14:33:26] Gotisch: hey :) have your problems been fixed now btw? [14:33:41] If you test a method in a set of tests that depends on other tests, without running those tests, well then your in for trouble .. me thinks.. [14:33:52] vHanda: *sob* you unsubscribed from the mailing list *sob* ;-) [14:35:06] tw, if you are in that weird file.. Please note testTokensAndRights, it starts the chain of dependencies [14:35:33] And it is a little weird .. I got a few comments on it.. ;) [14:36:29] jejebinks: well, the items are not there any longer when i get to testing site links. [14:36:39] maybe a test for removing items was added? [14:36:43] wouldn't that be fun?.... [14:36:58] It doesn't really do anything.. or test anything.. it just starts the dependency so a lot of tests that fails during debugging is turned off. [14:38:38] The real fun is that the tests must create an item just to check whats the top of the items before the tests are run [14:39:35] If debugging is turned on only a few tests and assertions are run before the testset completes [14:40:13] jejebinks: uh... and is that stuff marked as skipped, then? [14:40:17] because... [14:40:18] We should *really* use tear down, but for the moment (?) we can't delete items [14:40:27] JeroenDeDauw: doe you have api debugging turned on? [14:40:50] if i understand john correctlöy, that causes lots of tests to be skipped [14:40:59] which would explain why they passed for you while being broken for me [14:41:00] btw.. I have run the tests and they go green, but I have not pulled today [14:41:14] jejebinks: i'm not working on master [14:41:19] i'm working on the "simplification" patch# [14:41:43] it's totally borked as far as i can see. but i still don't understand why [14:44:31] Oki.. I don't know what you have included or changed in your version [14:44:56] Lydia_WMDE: definitely not waiting on a computer nor done. http://dpaste.com/758135/plain/ [14:45:12] jejebinks: if i can't figure it out, i'll cust post it for review, and let you have a look at it. [14:45:22] hm, maybe i should have done that an hour ago [14:45:40] jeremyb: heh i have to confess that this doesn't tell me much [14:46:08] Guess that could work.. Seems to be some patchsets on review now.. [14:46:10] * jejebinks hides [14:46:36] Lydia_WMDE: all 3 of the WMF auth NS say the same thing. and that thing is slightly different for www.wikidata vs. just plain wikidata. but both are in the end WMF IPs [14:46:55] jeremyb: mpfh - ok.... bad [14:46:59] thx [14:47:07] Is WBSettings changed in your setup? [14:47:14] * Lydia_WMDE diggs out the emails to figure out who to poke about that again [14:47:34] jejebinks: how do you mean? [14:48:08] testing of api depends on wbsettings to get som configuration.. [14:48:31] It should throw an error if it fails to find its settings [14:48:38] jeremyb: oh so it was you who i last talked to about that... i was already beginning to doubt myself ;-) anyway - what would be the next step to get this fixed? [14:49:22] Lydia_WMDE: well i filed an RT and then was told an RT was already existed. (by aude. she saw me mention it and said there already was one) [14:49:36] Lydia_WMDE: so maybe she knows? or i can poke someone [14:49:58] s/was // [14:50:00] jeremyb: hmmm there is one but it seems stalled and i can't see it to check [14:50:00] jejebinks: so, wblinksite seems to actually detroy the item. [14:50:19] and it doesn't seem to be moving [14:50:21] Lydia_WMDE: what's the #? [14:50:29] checking [14:50:43] destroy the item? eh.. o_O [14:50:45] jeremyb: 2919 [14:50:53] Lydia_WMDE: k, i'll poke [14:50:56] thx! [14:54:19] Uh, I believe you misunderstand me. WBSettings::get() will throw an error if it can't find the config vars [14:54:28] uuhhhhh. [14:54:33] guys? [14:56:08] ah, nvm [14:57:10] uuuggghhhh [14:58:22] * Lydia_WMDE hands Daniel_WMDE some baldrian ;-) [15:04:11] Daniel_WMDE: I do have it turned on yes [15:04:27] But this really should not affect the stuffs [15:04:32] Otherwise its broken [15:06:23] Debug turns off ode that is needed to do some of the tests [15:07:23] Itspossible to rewrite some of them, but I mostly just turned off tests that would fail as they are now [15:08:02] For example the tests checks handling of tokens, but if use of tokens are turned off its somewhat difficult to test them [15:08:15] I introduced that setting so you could make requests to the API without tokens. [15:08:29] The test can check this setting then no? [15:08:31] Its like testing the presence of a chicken when you squash the egg [15:10:22] The tests checks if the tokens user rights are used and skips the tests if they are not. [15:19:07] GRRREEEEN!!!! [15:19:09] omg. [15:19:26] and it was nearly exactly the same error as in getMultilangText. just in getSiteLinks. [15:19:36] it just took me forever to find it, because of the arcane effects. [15:20:19] :D [15:22:18] took me what, 3 hours? [15:22:26] but i got to stumbe around a lot of the code. [15:22:34] thats's a good thing. [15:23:22] The tests in setitems finds a lot of bugs, but they don't identify the root cause [15:23:58] Its just "something broke from the api and downwards in the stack [15:24:06] yea. [15:24:13] more input validation in more places would help [15:24:32] jejebinks, JeroenDeDauw: i'm testing a bit more, then i'll submit this. can you review it soon? perhaps we can get it merged tonight. [15:24:46] i'll be afk soon after submitting, but will probably be back on it later tonight [15:26:06] I'll take a look a little later [15:35:35] New patchset: Daniel Kinzler; "Simplified internal structure and get rid of methods that expose internal structure and modified api and tests to match" [mediawiki/extensions/WikidataRepo] (master) - https://gerrit.wikimedia.org/r/10727 [15:35:59] New patchset: Daniel Kinzler; "Simplified internal structure and get rid of methods that expose internal structure and modified tests to match" [mediawiki/extensions/WikibaseLib] (master) - https://gerrit.wikimedia.org/r/10728 [15:38:57] JeroenDeDauw: quick! the patches apply cleanly and are compatible! review before it's too late! [16:05:29] Daniel_WMDE: but I wrote most of the stuff [16:05:35] Although after 10 new patchsets... [16:06:58] Lydia_WMDE: you there? [16:13:15] JeroenDeDauw: Daniel_WMDE: can you find me a Lydia_WMDE ? [16:13:56] or can you comment on a discussion about a URI scheme that happened at the hackathon? [16:19:39] jeremyb: try luring one in with cookies, that usually works [16:19:49] damnit [16:20:03] Set-Cookie: channel=#wikimedia-wikidata [16:20:20] * JeroenDeDauw steals the cookies [17:02:42] New review: John Erling Blad; "If I run the tests included in this patchset the tests from Wikibase\Test\ApiSetItemTests goes red. ..." [mediawiki/extensions/WikidataRepo] (master); V: 0 C: 1; - https://gerrit.wikimedia.org/r/10727 [17:06:14] jeremyb: here now :) [17:06:59] hi [17:07:29] Lydia_WMDE: so...? [17:07:39] heh [17:07:42] what do you need? [17:07:48] 12 16:13:56 < jeremyb> or can you comment on a discussion about a URI scheme that happened at the hackathon? [17:07:50] uri scheme stuff? [17:07:51] hmmm [17:07:53] i wasn't there [17:08:00] Daniel_WMDE: you? [17:08:08] can you find out who was? ;-) ;-) [17:08:17] (you were there at the hackathon, right?) [17:08:30] i wasn't at the hackathon no - only the wikidata part before it [17:08:39] huh [17:09:09] aaaaanyway - i will try to find out more tomorrow - what do you want to know specifically? [17:09:40] so, sounds like something about choosing where the wikidata repo/client wikis will live long term [17:10:10] ohhhh [17:10:11] that [17:10:15] yeah [17:10:23] i will bring it up at scrum tomorrow [17:10:51] i don't understand why that's part of the discussion but it is. i asked to break off the static landing page to a different task and hte answer was essentially "that's not my decision to make" [17:11:24] -.- [17:11:26] i see [17:11:28] thx [17:12:01] anywaythe* [17:12:05] gah [17:12:05] the* [17:14:57]