[13:02:06] JeroenDeDauw: re the store, we want to use the same store object always as it has some in-memory caching [13:09:10] New patchset: Siebrand; "Use Linker::link() functions." [mediawiki/extensions/SemanticForms] (master) - https://gerrit.wikimedia.org/r/13003 [13:16:50] New review: Jeroen De Dauw; "Linker::link has only been static since 1.18, while SF is still keeping compat with 1.17 for now. So..." [mediawiki/extensions/SemanticForms] (master); V: 0 C: -1; - https://gerrit.wikimedia.org/r/13003 [13:17:36] nischayn22: typically we do yes [13:17:50] nischayn22: but at some point we might have a use case where this is not true [13:18:01] where we might want to have another store or so [13:18:38] For instance if we had some "syncronize stores" script, then there would be more then one [13:19:10] I'm not against having a singleton for the store, but think it makes sense from an OO perspective to pass it to the DI handlers [13:19:25] Can always ask Markus for a third opinion on this if you want [13:39:25] JeroenDeDauw: Ok, will confirm with Markus [13:39:44] JeroenDeDauw: and about the interface, why would we still need the base class? [13:40:10] just to create instances of subclasses? [13:42:16] New review: Siebrand; "Argh. I though this happened ages ago..." [mediawiki/extensions/SemanticForms] (master); V: 0 C: 0; - https://gerrit.wikimedia.org/r/13003 [13:55:31] nischayn22: can you has a look at my inline comment here? https://gerrit.wikimedia.org/r/#/c/12987/ [13:55:40] !gerrit [13:55:46] !gerrit 123 [13:55:52] @commands [13:55:53] Commands: channellist, trusted, trustadd, trustdel, info, configure, infobot-link, infobot-share-trust+, infobot-share-trust-, infobot-share-off, infobot-share-on, infobot-off, refresh, infobot-on, drop, whoami, add, reload, suppress-off, suppress-on, help, RC-, recentchanges-on, language, infobot-ignore+, infobot-ignore-, recentchanges-off, logon, logoff, recentchanges-, recentchanges+, RC+ [13:56:06] @info [13:56:06] http://bots.wmflabs.org/~wm-bot/dump/%23semantic-mediawiki.htm [13:57:33] JeroenDeDauw: Yes, I am looking into that, its basically just a hack [13:57:57] JeroenDeDauw: did you look my question about interfaces ? I think you got disconnected [13:58:07] !gerrit is https://gerrit.wikimedia.org/r/#q,$1,n,z [13:58:08] Key was added [13:58:10] 19:09 JeroenDeDauw: Ok, will confirm with Markus [13:58:10] 19:09 JeroenDeDauw: and about the interface, why would we still need the base class? [13:58:10] 19:10 just to create instances of subclasses? [13:58:13] !gerrit 1234 [13:58:13] https://gerrit.wikimedia.org/r/#q,1234,n,z [13:59:34] nischayn22: the base class could hold stuff such as the common store field [14:00:01] Right now it's not terribly useful, but having it is likely to save recftoring hassle later om [14:00:03] *on [14:00:17] JeroenDeDauw: I am not passing the store field anymore, because in some places we don't have one to pass [14:00:48] so I the Handlers create one when needed, and that happens only for Wikipages and container classes [14:15:10] nischayn22: right ok - let's leave it like that for now then [14:15:52] JeroenDeDauw: does having abstract class instead of Interface cause lesser efficiency? [14:16:33] then we could use the interface and have a Factory class or Base class for creating the objects [14:20:08] New patchset: Nischayn22; "Completely removed the CompatibilityHelpers" [mediawiki/extensions/SemanticMediaWiki] (storerewrite) - https://gerrit.wikimedia.org/r/12987 [14:20:25] nischayn22: using the interface is not performance related [14:20:32] to be honest, I don't know which is faster [14:20:36] But that really does not matter [14:20:42] as the difference is way to trivial [14:20:49] This is about having a proper design [14:20:58] JeroenDeDauw: I read somewhere that interfaces are faster [14:21:07] ofc its about design [14:21:36] Type hinting against actual classes sort of goes against the principles of dynamic languages if you ask me [14:21:46] And type hinting is good [14:21:52] Catches lots of errors [14:23:53] New review: Jeroen De Dauw; "To be honest, I still don't understand what this code is doing - can you clarify further? Else I'll ..." [mediawiki/extensions/SemanticMediaWiki] (storerewrite); V: 0 C: 0; - https://gerrit.wikimedia.org/r/12987 [14:25:19] JeroenDeDauw: I can explain more, but I haven't tested them myself [14:25:35] Actually I can't think of how to test them [14:36:20] nischayn22: my problem with that code is that it's not clear to me what it does [14:36:29] I can read the code of course [14:36:41] I know what it does in the sense that I understand the instructions [14:36:58] But not in the sense of what it actually means - more like why this is being done [14:37:32] JeroenDeDauw: Initially it used getDBkeysforDataItem and then made the where statement [14:38:18] the result was an array without keys, so it then had to use the proptable to get the table fields and then do the mapping [14:39:06] now, it gets array with fields as keys For example array('oid'=>12) and doesn't have to do the mapping [14:39:23] however for the valueIndex it has to do that using a hack [14:39:32] JeroenDeDauw: Hope you got somewhat of it ;) [14:50:08] New patchset: Nischayn22; "Fixed formatting and var names for change 12981" [mediawiki/extensions/SemanticMediaWiki] (storerewrite) - https://gerrit.wikimedia.org/r/13020 [14:54:19] New patchset: Nischayn22; "Completely removed the CompatibilityHelpers" [mediawiki/extensions/SemanticMediaWiki] (storerewrite) - https://gerrit.wikimedia.org/r/12987 [15:02:13] New review: Jeroen De Dauw; "(no comment)" [mediawiki/extensions/SemanticMediaWiki] (storerewrite); V: 1 C: 2; - https://gerrit.wikimedia.org/r/13020 [15:02:15] Change merged: Jeroen De Dauw; [mediawiki/extensions/SemanticMediaWiki] (storerewrite) - https://gerrit.wikimedia.org/r/13020 [15:17:26] New review: Yaron Koren; "Siebrand - this change looks great! I'm looking forward to getting it into the code. And thanks for ..." [mediawiki/extensions/SemanticForms] (master); V: 0 C: 0; - https://gerrit.wikimedia.org/r/13003 [15:26:42] nischayn22: so do you still have stuff to do? [15:46:42] JeroenDeDauw: No actually, will discuss tomorrow with Markus [16:51:02] New review: Siebrand; "@Yaron: feel free to upload a new patch set that can be merged. That would make me very happy." [mediawiki/extensions/SemanticForms] (master); V: 0 C: 0; - https://gerrit.wikimedia.org/r/13003 [19:41:14] Dantman Actually this is not a SMW related question. You coded so many cool extensions. Why not move them to Git? However I guess there must be a reason for you not to do it. [19:41:36] I'm just lazy [19:42:00] I havent made any updates to them so I've had no reason to bother migrating [19:42:19] Plus I've been too busy to do MW stuff anyways [19:42:57] Ah ok. Fair enough. Just take your time. :) They are still there, aren't they? ;) [20:23:38] hi [20:48:38] hi