[00:25:08] any thought on whether https://en.wikipedia.org/wiki/Data_transfer_object should be a subclass of design pattern or of object? (slight amusement at the thought of something being a subclass of an object) [00:36:35] in some languages everything is a subclass of an object [00:37:49] subclass of an object or subclass of a class...? last I checked, objects are discrete instantiations of classes, by definition. [00:37:52] * Sky2042 could be wrong. [00:38:48] http://msdn.microsoft.com/en-us/library/system.object.aspx [00:38:55] "Supports all classes in the .NET Framework class hierarchy and provides low-level services to derived classes. This is the ultimate base class of all classes in the .NET Framework; it is the root of the type hierarchy." [01:00:31] So does that imply that it's a class or an object...? [01:28:50] anyone here speak vietnamese? [01:31:26] No, but I just learned I live in Nam Dakota. [06:41:08] legoktm: now i am [06:41:21] oh hey [06:41:26] hey :) [06:41:59] I was looking at https://wikimania2013.wikimedia.org/wiki/Schedule#Saturday.2C_August_10 and it seems they scheduled the "Ask us anything about Wikidata" and "State of Wikidata" at the same time [06:42:21] wtf? [06:42:23] so i poked James_F|Away about it in #wikimania [06:42:27] ok [06:42:30] idk if anything got done about it though [06:43:02] ohhh [06:43:02] no [06:43:13] it is in the same box in the schedule there [06:43:22] but that means they are in the same room one after the other [06:43:39] ohohoh [06:44:02] thats good then :D [06:44:17] though, doesnt it make more sense to do the Ask us panel at the end? [06:44:33] very much so [07:16:46] Lydia_WMDE legoktm and it's a wiki .... fixed :) [07:16:56] \o/ [07:16:58] thx aude [07:17:02] :D [07:17:03] thanks [07:17:05] makes sense to me [07:44:01] New patchset: Henning Snater; "Add .jshintrc for Wikibase and make it pass" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/64472 [08:00:59] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66269 [08:07:09] New patchset: Henning Snater; "Inject value formatters into EntityView instead of using global in EntityView" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66270 [08:34:20] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66270 [08:41:53] New patchset: Aude; "(bug 49105) Add toolbox link and js config var for connected wikibase item" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66908 [08:52:27] New patchset: Henning Snater; "Implemented EntityContent::getParserOutput" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66271 [09:02:12] New patchset: Henning Snater; "EntityView::getHtmlForClaims separation into getHtmlForClaim" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66274 [09:42:25] New review: Daniel Kinzler; "The approach is fine and the code looks good overall, but there are some architectural as well as so..." [mediawiki/extensions/Wikibase] (master) C: -1; - https://gerrit.wikimedia.org/r/65648 [09:43:43] DanielK_WMDE_: could you answer http://www.wikidata.org/wiki/Wikidata:Contact_the_development_team#Indication_of_calendarmodel_in_time_values maybe? [09:49:07] Lydia_WMDE: doing.... trying to find the correct bug :/ [09:52:58] Lydia_WMDE: done [09:55:02] DanielK_WMDE_: Back then I had newFromMove in the job class, Jeroen complained about it: https://gerrit.wikimedia.org/r/#/c/65648/3/lib/includes/UpdateRepoOnMoveJob.php [09:56:41] hoo|away: then i very much disagree with him [09:56:49] it's an alternative constructor. [09:56:53] it belongs exactly there. [09:57:20] :P Ok :) [09:57:31] hoo|away: but you can keep using the constructor directly, not a big deal. [09:59:26] I'll see... probably I'll let the testability decide (as I already have client unit tests) [10:03:43] New review: Daniel Kinzler; "(1 comment)" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66547 [10:05:46] New patchset: Tobias Gritschacher; "(bug 43870) new Special:SetSiteLink" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/45574 [10:06:42] DanielK_WMDE_: thx [10:08:27] aude: FIXED! https://gerrit.wikimedia.org/r/#/c/45574/ [10:08:51] New review: Aude; "see comment" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/45574 [10:08:59] see my comment [10:09:16] then i can try once i finish other stuff [10:12:36] New review: Daniel Kinzler; "uninitialized var." [mediawiki/extensions/Wikibase] (master) C: -1; - https://gerrit.wikimedia.org/r/66547 [10:14:42] New review: Aude; "(1 comment)" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/45574 [10:14:54] Tobi_WMDE: look again [10:15:10] gerrit eat my comments :) [10:19:09] New review: Tobias Gritschacher; "(1 comment)" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/45574 [10:23:56] New review: Aude; "(1 comment)" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/45574 [10:26:48] New patchset: Tobias Gritschacher; "(bug 43870) new Special:SetSiteLink" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/45574 [10:27:56] aude: I've added it. not-tested! [10:29:04] ok [10:29:17] if you don't test then i will try when i'm done with other stuff [10:29:59] aude: tested the valid cases, but did not test cases with errors [10:30:13] ok [10:30:15] so, please chack again as soon as you have time [10:30:23] ok [10:31:12] * aude looking at https://gerrit.wikimedia.org/r/#/c/66122/ and see if i can figure out why it's not running *all* the tests [10:32:39] how I mark some to be speedy removed? like this orphan talk page http://www.wikidata.org/wiki/Talk:Q4148644 [10:50:04] New patchset: Aude; "(bug 45244) show correct diff revision id in diff view" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66547 [10:50:08] New review: Tobias Gritschacher; "(1 comment)" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [10:52:42] linneawp: [[Wikidata:Requests for deletions]] :) [10:52:44] [1] 10https://www.wikidata.org/wiki/Wikidata:Requests_for_deletions [10:53:28] i'll delete that page for you, though [11:05:31] linneawp: or you can also put {{delete}} to the talk page. [11:05:32] [2] 10https://www.wikidata.org/wiki/Template:delete [11:08:48] New patchset: Aude; "Factor EntityDataRequestHandler out of special page" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [11:09:05] DanielK_WMDE_: https://gerrit.wikimedia.org/r/#/c/66122/ [11:09:21] i don't think phpunit liked the static provider function the way it was [11:09:46] i split it into another class that special entity data and the request handler tests can use, and that works [11:12:25] New patchset: Henning Snater; "Basic EntityView PHPUnit test" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66957 [11:17:35] New review: Aude; "looks fine and all the special pages for editing work good." [mediawiki/extensions/Wikibase] (master) C: 2; - https://gerrit.wikimedia.org/r/45574 [11:17:41] \o/ [11:18:44] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/45574 [11:29:35] aude: now, that Special:setSiteLink is merged, please also merge https://gerrit.wikimedia.org/r/#/c/66399/ [11:29:40] ok [11:29:51] i have a follow up for site link and will do tests [11:31:11] New patchset: Aude; "do null check for invalid site, instead of false" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66958 [11:31:29] Tobi_WMDE: https://gerrit.wikimedia.org/r/#/c/66958/ [11:33:42] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66399 [11:36:00] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66958 [11:47:13] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66957 [11:47:14] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66274 [12:05:13] New review: Daniel Werner; "(1 comment)" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66267 [12:06:27] New patchset: Daniel Werner; "Moves wgValueFormatters global definition in MW specific initialization file" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66243 [12:06:57] New patchset: Daniel Werner; "Made DataValue::getType static, added tests for both static/instance usage" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66267 [12:12:27] New review: Daniel Werner; "The time.parse should be renamed into time.Parse, not deleted, for diffing and git blame's sake." [mediawiki/extensions/DataValues] (master) C: -1; - https://gerrit.wikimedia.org/r/66546 [12:21:34] Danwe_WMDE: are php value formatters already used anywhere? [12:21:52] * aude not danwe but used for entity id formatting [12:22:09] there is a string one (which really doesn't do much) [12:22:23] yea, i mean the wgValueFormatters registery, via ValueFormatterFactory [12:22:29] hmmm [12:22:29] doesn't look like it [12:22:33] DanielK_WMDE_: we do [12:22:38] where? [12:22:44] well, we register the entity id formatter [12:22:49] with the setting [12:22:55] DanielK_WMDE_: Last week I submitted a change set to use them if available in the EntityView [12:23:00] hm, why does Wikibase have a ValueFormatterFactoryTest of ValueFormatterFactory is defined in DataValues? [12:23:07] not sure this is merged but I can't see it on gerrit right now [12:23:22] I don't know [12:23:24] Danwe_WMDE: what would i grep for? [12:23:37] aude: yea - i want to test that that works :) [12:23:37] Also, the formatters used in another place, some evil class of Jeroen's, one moment [12:24:19] yes it is used to do evil :( [12:24:51] DanielK_WMDE_: EntityView::getHtmlForClaim [12:25:02] ah, git pull helps :P [12:25:21] DanielK_WMDE_: and TypedValueFormatter (that's the evil one) [12:25:29] got to go [12:25:34] thanks [12:29:45] Danwe_WMDE: 51° 20' N, 12° 25' O -> "kein gültiger Wert erkannt" [12:31:06] arg. [12:31:36] Tobi_WMDE: got any valid geo-coordinates for me? i need to create at least one value with the geo type :) [12:31:46] how about decimal degress? [12:31:58] 51°N 12°O <--- still invalid [12:32:04] 51.40.7142° N, 74.0064° W [12:32:41] nope [12:32:43] 51.4071, 74.0064 [12:33:01] or -74.0064 fo [12:33:03] ok, that works! [12:33:05] thanks [12:33:05] r W [12:33:30] -30.56327 for south [12:33:41] haha, this doesn't even round trip. [12:33:46] oooh [12:34:25] ...and it doesn't save?! [12:34:29] what am i missing? [12:34:50] (no error details) [12:34:53] * aude not actually tried [12:34:54] it [12:34:58] * aude should [12:35:37] Catchable fatal error: Argument 1 passed to DataValues\GeoCoordinateValue::newFromArray() must be of the type array, string given [12:35:44] right [12:35:46] it's like time? [12:35:59] time is an array of timestamp, calendar and other stuff [12:36:13] i suppose [12:36:17] also it might not be doing json decode right [12:36:20] or it should [12:36:33] you doing this via the api/ [12:37:11] i'm trying to save it [12:37:18] hmmmm [12:37:19] so yes, api [12:37:26] it could be buggy [12:37:26] seems pretty broken in master [12:37:35] we fixed it for qualifiers to decode correctly [12:37:38] buggy? "unusable" is more like it :P [12:37:41] this should *just* work [12:37:45] which api moduel? [12:37:47] module? [12:38:21] wbsetclaim [12:38:27] ok, it should work [12:38:31] {"type":"statement","mainsnak":{"snaktype":"value","property":"p33","datavalue":{"type":"geocoordinate","value":"51.4071|74.0064"}},"id":"q188$2325ab04-4d4a-b712-3c8a-ac8251888654","qualifiers":{},"rank":"normal"} [12:38:43] it's not even an array [12:38:52] ah, maybe that's it [12:39:00] the code expects an array, but is getting a string [12:39:07] * aude tries [12:39:44] yes the value is bad [12:39:48] just from looking [12:39:57] * aude tries [12:40:44] i expect it needs a json encoded array of latitude, longitude and altitute (optional i hope) and globe (optional, defaults to earth) [12:41:28] new from array return new static( $data['latitude'], $data['longitude'], $data['altitude'], $data['globe'] ); [12:41:59] yes. [12:42:18] but it'S getting a string [12:42:45] hmmm [12:42:48] aude: this should not cause a fatal error, the structure must be checked first, so an error can be reported nicely [12:43:04] regardless if you are doing it right or not, we have a bug for this [12:43:08] * aude raged about it [12:43:10] my guess is: some update to the JS code hasn't been merged yet [12:43:18] (or was forgotten in a commit) [12:43:21] js shouldn't matter [12:43:37] https://bugzilla.wikimedia.org/show_bug.cgi?id=48888 [12:44:00] right [12:44:11] i think it is a general issue [12:44:23] yea [12:44:27] on top of poor documentaiton [12:44:38] we are making it a challenge for bots [12:46:34] New review: Henning Snater; "Your altering original PEG.js code even more which should be avoided. The TODOs should be in the fil..." [mediawiki/extensions/DataValues] (master) C: -1; - https://gerrit.wikimedia.org/r/66288 [12:48:07] New review: Daniel Kinzler; "Seems appropriate, and the registration works." [mediawiki/extensions/DataValues] (master) C: 2; - https://gerrit.wikimedia.org/r/66243 [12:48:21] Change merged: jenkins-bot; [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66243 [12:49:59] New review: Henning Snater; "(1 comment)" [mediawiki/extensions/DataValues] (master) C: -1; - https://gerrit.wikimedia.org/r/66299 [12:53:39] DanielK_WMDE_: sure you got it now but [12:53:41] datavalue":{"value":{"latitude":40.67,"longitude":73.94,"altitude":null,"globe":null},"type":"geocoordinate"}} [12:53:44] works for me [12:55:40] aude: that'S not the json i'm getting... strange [12:55:49] hmmmm [12:56:36] jsonencode( array( 'latitude' => 40.67, 'longitude' => 73.94 ) ) [12:56:40] + the type [12:56:58] my bot sends that as the value [12:57:15] yes, but apparently, the UI doesn't [12:57:16] now, 40.67 cannot be enclosed as a string [12:57:23] the ui does not work yet [12:57:29] New review: Henning Snater; "Needs tests." [mediawiki/extensions/Wikibase] (master) C: -1; - https://gerrit.wikimedia.org/r/66271 [12:57:31] duh! [12:57:36] that'S what i just found out :P [12:57:47] it's however the same error we got with invalid calendarmodel [12:57:52] which the api *does* accept [12:57:55] and not validate [12:58:03] and then it displays same way in the ui [12:58:29] https://bugzilla.wikimedia.org/show_bug.cgi?id=48965 [13:01:12] DanielK_WMDE_: geocoordinates are not finished yet.. there are still changes on gerrit, see https://gerrit.wikimedia.org/r/#/q/owner:%22Daniel+Werner%22+status:open,n,z [13:01:15] errm, i can's save "string" values either, it seems [13:01:18] * DanielK_WMDE_ is confused [13:01:31] strings are just strings but need to be json encoded [13:01:34] i think [13:01:43] q [13:01:46] Tobi_WMDE: i know they are not finished, but i thought i would be able to at least save them from the UI [13:01:52] json_encode( $value ); [13:01:58] $value being a string in that case [13:01:59] aude: no, in the UI [13:02:02] oh [13:02:05] we have that live [13:02:10] what? [13:02:14] if something in the API now broke editing for strings, that would be bad [13:02:23] wtf? [13:02:36] maybe something wonky on my system, don't panik just yep :) [13:02:41] you mean on master? [13:02:43] or on wikidata? [13:02:47] but please test that you can save values for "string" type properties [13:02:51] * aude panics [13:03:39] aude: let me finish the diff review, then i'll have another poke at it [13:03:43] ok [13:04:04] strings work fime [13:04:05] fine [13:04:37] the "save" option stays grey for me [13:04:47] weird [13:04:55] anything in the console? [13:05:43] DanielK_WMDE_: works fine for me [13:06:05] I see no probs [13:06:52] just coordinates not working until everything is merged. the UI is kind of ready, but in the backend there are still some 5, 6 changes pending [13:06:53] yea, i guess i got something muddeled here. [13:07:07] Tobi_WMDE: trying to get to that# [13:07:29] \o/ [13:07:36] DanielK_WMDE_: think Henning_WMDE currenty reviewing that [13:11:38] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66547 [13:13:13] New patchset: Henning Snater; "Add .jshintrc for Wikibase and make it pass" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/64472 [13:20:56] Change merged: Tobias Gritschacher; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/64472 [13:28:27] New review: Jeroen De Dauw; "(1 comment)" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66299 [13:35:24] New patchset: Tobias Gritschacher; "Moving entity operations into separate ChangeOps" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66392 [13:44:19] * jeblad casts a woodoo-spell over Oracle :( [13:46:27] !nyan [13:46:27] ~=[,,_,,]:3 [13:59:50] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66392 [14:05:19] New patchset: Aude; "add permission, block and read only checks for Special:ModifyEntity" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66970 [14:05:49] New review: Daniel Werner; "Since it got altered in non-trivial way already and to be alterned to be a coordinate parser rather ..." [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66288 [14:07:48] Change merged: jenkins-bot; [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/65647 [14:07:48] New review: Daniel Werner; "(1 comment)" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66299 [14:08:09] New patchset: Daniel Werner; "Coordinate values of backend and frontend can now exchange precision" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66299 [14:12:07] !nyan [14:12:07] ~=[,,_,,]:3 [14:12:11] really? [14:12:13] lol [14:13:05] DanielK_WMDE_: why do we have a "EntityContentTest" but no "AbstractContentTest" ? [14:16:23] Danwe_WMDE: because there isn't much you could test about a Content object without knowing anything specific about the content model [14:16:58] Danwe_WMDE: what do you want to use it for? [14:17:21] DanielK_WMDE_: that's how many of our abstract tests work, no? A specific test implementation would provide this kind of information [14:20:37] DanielK_WMDE_: just to have that one in core for usage in other content models [14:21:04] I am implementing a testGetParserOutput since it is currently missing so I noticed that [14:22:17] Danwe_WMDE: in some cases that may be useful, but tests for Content tend top be pretty simple. Probably easier to just write them streight forward [14:22:55] perhaps [14:23:01] also, base classes are annoying for this... I'd really like to be able to have "test components" that can be combined into an actual test... one such component for each interface an object implements, for instance. [14:30:21] New review: Daniel Kinzler; "Seems good overall, but has some small issues/unclarities." [mediawiki/extensions/Wikibase] (master) C: -1; - https://gerrit.wikimedia.org/r/66295 [14:31:22] DanielK_WMDE_: sounds good, that's what I am trying to do in JS as well [14:32:08] DanielK_WMDE_: anyhow, if I am testing the EntityContent::getParserOutput, does it matter what the Title object given to the EntityContent is? [14:36:09] seems to always work [14:36:09] New patchset: Daniel Werner; "Added Wikibase\EntityContent::testGetParserOutput" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66971 [14:37:30] New review: Daniel Werner; "Added test in I57ffd02. This was a missing test in the abstract EntityContentTest, so I created an i..." [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66271 [14:54:34] New patchset: Aude; "Move code for injecting the edit/add links link into class and add tests" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66295 [14:57:30] New patchset: Henning Snater; "Adding .jshinrc for DataValues and making it pass" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66974 [15:00:42] aude: hm, your changes to https://gerrit.wikimedia.org/r/#/c/66122 mean quite a bit of refactoring for https://gerrit.wikimedia.org/r/#/c/66245 [15:01:32] hmmmm [15:01:45] factoring out EntityDataTestProvider.php seems like a good idea, but adding that as another change at the end of the chain would have made things easier [15:01:55] it's up to you to figure out if there is a better way [15:02:41] aude: but that would mean undoing your amend, which would probably make the mess worse. [15:02:51] i'll just refactor. [15:03:02] you can take the first patch [15:03:05] aude: btw... now since we both worked on that patch, who#s going to review it? [15:03:27] New patchset: Henning Snater; "Adding .jshinrc for DataValues and making it pass" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66974 [15:03:27] then just note if you factor out the static method then it seems to work [15:04:05] or stuff like injectIds can come out of the entity data request handler test [15:05:02] * aude doesn't totally understand why the static method was a problem [15:07:47] aude: oh? the static method was a problem? err... which one exactly? [15:07:54] the data provider [15:08:24] but there still is a static data provider method, just a different one? and that works? [15:08:44] i tried referencing it via a different non-static provider for the test [15:08:47] it didn't work [15:08:57] hrm [15:09:00] you should check [15:09:47] works [15:09:51] ok [15:09:54] strange [15:10:18] New patchset: Henning Snater; "Adding .jshinrc for DataValues and making it pass" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66974 [15:10:21] and it was fine when accessed from special entity data test [15:14:42] get myself into a complete mess now. [15:14:47] no [15:14:49] rebase seems top work fine, but gerrit got confused [15:14:59] now i'm not sure how to fix this [15:15:03] probably do it over or something :/ [15:15:13] it's not that much code [15:15:27] took a while to sort out why the test didn't run [15:15:40] or how to get them to run [15:15:56] * aude can try to rebase if you like [15:18:09] DanielK_WMDE_: it worked [15:18:28] * aude can upload new patch [15:20:51] aude: sure, go ahead [15:21:03] k [15:21:14] aude: when i rebased, gerrit wanted toi upload *three* changes (instead of two) [15:21:15] oh no [15:21:16] no idea where it got the third one from [15:21:28] New patchset: Aude; "Factor EntityDataRequestHandler out of special page" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [15:21:29] New patchset: Aude; "Handle If-Modified-Since in Special:EntityData" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66245 [15:21:33] done [15:21:54] what i did and what 66245 touches are different parts of code [15:22:00] clean rebase [15:22:05] tests pass [15:23:37] New review: Daniel Kinzler; "aude's update looks fine." [mediawiki/extensions/Wikibase] (master) C: 1; - https://gerrit.wikimedia.org/r/66122 [15:24:51] aude: err, EntityDataRequestHandlerTest should be using the same test cases as SpecialEntityDataTest.php [15:25:03] it should [15:25:04] seems like it's now duplicating the code? [15:25:11] hmmm [15:26:43] this looks weird [15:28:13] ok, i need to rebase again [15:28:23] it eliminated what i did [15:28:36] wtf? [15:28:36] right... kind of what happened to me [15:28:58] and when i finally got the code looking right, gerrit got confused >_< [15:29:11] looks better [15:29:39] ok got it [15:29:42] one minute [15:31:41] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66295 [15:32:13] New patchset: Aude; "Factor EntityDataRequestHandler out of special page" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [15:32:14] New patchset: Aude; "Handle If-Modified-Since in Special:EntityData" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66245 [15:32:34] New patchset: Daniel Werner; "Implemented time.js parser constructor" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66546 [15:32:37] DanielK_WMDE_: looks better now [15:33:01] just verify that all the tests are getting the test data from the right place [15:33:24] * aude thinks they are [15:34:51] New review: Daniel Kinzler; "aude: thanks for the update!" [mediawiki/extensions/Wikibase] (master) C: 1; - https://gerrit.wikimedia.org/r/66245 [15:35:04] :) [15:35:09] aude: so... who's going to review the EntityData stuff now that we have both touched it? [15:35:16] Change merged: jenkins-bot; [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66970 [15:35:21] probably can ask tobi [15:35:28] would be nice to have it in tomorrow. not critical though [15:35:29] but i only moved stuff around [15:35:36] i think i could still review [15:35:45] if tobi is not interested [15:35:54] New patchset: Daniel Werner; "Implemented time.js parser constructor" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66546 [15:36:04] hm... yea, i guess you can go ahead. [15:36:16] ugh! no! [15:36:17] i did add @covers [15:36:24] now jenkins is complaining about the latest changeset! [15:36:25] huh? [15:36:48] PHP Fatal error: Call to protected method Wikibase\Test\EntityDataRequestHandlerTest::injectIds() from context 'Wikibase\Test\SpecialEntityDataTest' [15:36:49] ok, ok need to check that everything is in order after rebase [15:36:56] hmmm [15:36:58] ok [15:37:11] easy enough to fix [15:37:14] just make it public [15:39:43] curious how it ever worked? [15:40:10] EntityDataRequestHandlerTest::injectIds( $subpage, $item ); [15:40:11] ? [15:40:14] anyway, fixing [15:40:49] New patchset: Aude; "Handle If-Modified-Since in Special:EntityData" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66245 [15:40:56] new patch [15:41:57] New review: Jeroen De Dauw; "Some scary code in there. Needs simplification before I can review this and be confident it does not..." [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [15:44:07] oh ffs. [15:44:38] New review: Aude; "@jeroen if this patch does much more factoring out than this, then i think the patch is doing too mu..." [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [15:45:07] * aude disagrees with jeroen there [15:45:27] i don't want a gigantic monster patch that does a bazillion refactorings [15:46:21] boring [15:46:29] heh [15:47:05] New review: Jeroen De Dauw; "To clarify: the scariness comes from high complexity combined with similarly complex tests. The code..." [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [15:47:15] New patchset: Daniel Werner; "Implemented time.js parser constructor" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66546 [15:48:23] New patchset: Daniel Werner; "Implemented time.js parser constructor" [mediawiki/extensions/DataValues] (master) - https://gerrit.wikimedia.org/r/66546 [15:48:48] Reedy: boring? i got something for you!" [15:49:01] Reedy: https://gerrit.wikimedia.org/r/#/c/28199/ [15:49:05] it's starting to smell :P [15:49:19] aude: i'll address (some of) jeroens concerns. [15:49:24] DanielK_WMDE_: ok [15:49:55] New review: Jeroen De Dauw; "aude: doing this in a follow up is fine. However doing it "later" is not going to happen - we have s..." [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [15:50:00] Is just pressing the rebase button enough? [15:50:32] Just press the button and run! =P [15:51:19] Reedy: i hope not :) [15:55:25] New review: Daniel Kinzler; "@jeroen: i can add a follow-up that factors out some more stuff; I don't think it's worth blocking o..." [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [15:56:15] New review: Daniel Kinzler; "re" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [16:15:45] DanielK_WMDE_: aude: anybody has an idea why we have this "exclude" parameter in the editEntity API module? I know what it does but I don't see any real use case for it.. [16:16:09] Tobi_WMDE: no idea [16:16:55] Tobi_WMDE: i'd say kill it, but i havn't really investigated. also, it needs to be documented as a breaking change [16:17:01] ok, one could edit an entity, excluding "description" or something? [16:17:11] do people really use it that way? [16:17:24] aude: that's exactly my question.. do peaople use that [16:18:18] you could ask on wikidata-tech or poke some bot people here [16:19:05] if you changed e.g the description why would you want to exclude it & if you don't change it, why would you send it or why would you have to exclude it [16:19:36] no idea [16:20:08] * aude curious :) [16:20:12] Tobi_WMDE: ask jeblad [16:22:19] thanks Reedy! [16:22:51] alright, the sun is blazing on me and telling me to go home :) [16:22:57] DanielK_WMDE_: see you here tomorrow? [16:23:42] i'll poke at the set site link page more to ensure no isues and work on tests and continue on my other todo items [16:23:47] no issues [16:25:10] aude: yea, i'm there tomorrow [16:30:10] New patchset: Daniel Kinzler; "Remove global state from EntityDataRequestHandler" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66980 [16:31:15] New review: Daniel Kinzler; "addressed several concerns in follow-up Id902acdf3" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66122 [16:38:27] Reedy: here's an easy one: https://gerrit.wikimedia.org/r/#/c/46945/ [16:52:19] New patchset: Tobias Gritschacher; "Split up huge and ugly switch-case into separate methods (WIP)" [mediawiki/extensions/Wikibase] (master) - https://gerrit.wikimedia.org/r/66981 [16:53:21]