[00:20:17] TrevorParscal: do i need any extensions other than AFT, EmailCapture and SimpleSurvey for AFT development? [00:20:38] PrefSwitch [00:21:13] it will be automatically included for you though... [00:21:24] oh? [00:21:39] because it's a dependency of SimpleSurvey [00:21:47] oh ok [00:21:48] cool [00:22:19] strike that reverse it [10:33:08] RoanKattouw: switch here? [10:33:13] cool [10:33:17] I don't have it checked out yet no [10:33:25] no stress about disappearing, I have other work to do as well :) [10:34:00] It'd really just be 10 mins of unresponsiveness while I make my lunch that I would then eat at my computer, so no biggie [10:34:17] :) cool [10:34:37] alright, so check out svn+ssh://svn.wikimedia.org/svnroot/mediawiki/branches/lqt-rewrite/extensions/LiquidThreads [10:34:39] OK minor nitpick #1: file names drop prefixes [10:34:54] I don't care much, others might [10:35:09] (talking about how e.g. LiquidThreadsChannel is implemented in Channel.php) [10:35:23] yeah, I did that on purpose for tab-completion and what not [10:35:31] Ah right [10:36:17] the schema that's been implemented is in /new-schema.sql [10:36:21] Do you know how PHP lets you do new self(); ? [10:36:55] *RoanKattouw is just dumping his thoughts onto IRC, apologizes for unstructuredness [10:37:10] I didn't know that [10:37:17] I tried new __CLASS__ [10:37:23] but that didn't work for me [10:38:36] That works but only with $foo = __CLASS__; new $foo(); [10:38:46] new self() isn't really obvious and I'm not sure if and where it's documented [10:39:09] you notice, of course, that Post and Topic both have revision management code, and it's virtually identical. I'd like to refactor that into VersionedObject and VersionObject eventually [10:39:21] notice that all classes inherit from one of the two, so it can happen slowly [10:39:38] and some of the database loading/saving code can go upstream into LiquidThreadsObject [10:40:01] Right [10:40:15] but honestly I just want to push ahead now, and it's not trivial [10:40:18] Yeah the DB stuff is all different tables and fields but that can be dealt with [10:40:25] right [10:40:40] abstract function getVersionField(); [10:40:45] abstract function getDatabaseTable() [10:40:46] etc [10:41:17] Aye [10:41:28] With getRow(), you've already factored it in a way that makes this easier [10:42:44] And it would give Object some meat, which I notice right now only has getID(0 [10:44:37] What's the storage format for ltv_user_ip? Just text [10:44:38] ? [10:47:11] werdna: Why do both topic and topicversion have a channel field? Can a topic change its channel association over time? [10:47:58] sorry, i'm tethering [10:48:00] and it just broke [10:48:04] okay [10:48:16] ltv_user_ip is just a varchar [10:48:16] No worries [10:48:33] a topic can indeed change its channel association over time [10:48:38] Oh right, got thrown off by the 'binary' part but everything else is also binary [10:48:42] OK [10:48:46] I can decide that X topic doesn't really belong on this page, it should actually go to this other one [10:48:49] and it can be moved [10:48:59] ltv_summary and channel aren't documented, although they're fairly obvious to me [10:49:13] nod [10:49:15] What I don't understand very well is the user_id,user_ip index [10:49:31] on which table? [10:49:37] topic version or topic? [10:49:39] topic version [10:49:46] topic has no user fields [10:49:51] changes by a given user [10:49:59] it's like Special:Contributions/User [10:50:04] Yeah but you'd want to sort those by timestamp, right? [10:50:10] probably, yes [10:50:29] So I guess that means it needs ltv_timestamp added at the end [10:50:35] yep [10:50:38] added to the list [10:50:38] And I'll need to look up how MySQL handles null values in indices [10:51:15] I've inserted a few trial entries [10:51:25] it doesn't throw an error, at least [10:52:49] Aha [10:52:51] A search using col_name IS NULL employs indexes if col_name is indexed. [10:53:00] No, I wasn't worried about failures, just about optimization [10:53:13] right [10:53:26] You'd have queries like WHERE user_id=123 AND user_ip IS NULL ORDER BY timestamp [10:53:59] The is null clause is redundant but important because it allows MySQL to use the index [10:55:22] 85 -- Everything below this is a cache of the content of the current version. [10:55:23] 86 -- It's here to simplify queries. [10:55:29] Followed by ... nothing :) [10:56:41] topic and parent post? [10:57:27] Oh ,right [10:57:32] I forgot those are both versioned attributes [10:57:34] My bad [10:57:50] :) [10:57:59] Why are poster_id and _ip versioned attributes, though? [10:59:32] And I guess you may want to have a user,timestamp-like index on post_version as well, if you have a UI feature that does Special:Myposts or something [10:59:55] well, they're versioned because at some point in the future we might want to be able to reattribute posts. [11:00:17] and it seems like a simple design decision I can take to make things a lot more flexible [11:00:26] I haven't heard any arguments against it [11:00:29] Good point [11:00:53] I guess we should be careful to say "meh, no one's gonna use that anyway", we know better now :) [11:00:59] yeah :; [11:01:00] :p* [11:01:04] that was kinda my thinking [11:01:33] Alright, schema looks good [11:01:40] :) [11:01:45] with those caveats [11:01:52] Yeah [11:02:05] Which is basically "add one column to one index" [11:02:14] also, adding a user,timestamp index on post_version [11:02:18] If you need it, yeah [11:02:26] Moving on to the code [11:02:28] In Post.php: [11:02:30] 40 * Don't use this method externally, use one of the other factory methods. [11:02:34] Then why is the method public? :) [11:02:41] I'm... not.. sure? [11:03:16] I think that line should be removed, or reworded [11:03:54] It's for loadFromConditions() [11:04:09] Either that line is a lie and should be fixed, or the method should be made protected [11:04:10] yeah, I know [11:04:17] well, it's too strong [11:04:20] The latter makes more sense to me offhand but you know the code better [11:04:41] Ideally people would usually use factory methods [11:04:48] but there might be some cases where special conditions are usable [11:05:16] Also, the $fetchText parameter is undocumented and even unused [11:05:45] ah yes, good catch [11:06:57] Hmm and LQTPost doesn't inherit from anything [11:07:53] *werdna adds to list [11:09:56] LQTPostVersion same [11:11:33] hey ^demon [11:13:14] hmm: [11:13:17] 394 $parent_topics = array( $post->getTopicID() ); [11:13:18] 395 $parent_topics[] = $post->getPendingVersion()->getTopicID(); [11:13:20] 396 $parent_topics = array_unique( $parent_topics ); [11:13:40] You can also do $parent_topics = array_unique( array( $foo, $bar ) ); [11:13:41] <^demon> werdna: Hey [11:14:40] wassup? [11:15:43] <^demon> Still half asleep :p [11:15:56] so, not you [11:21:34] Hmm I think there's an edge case in LQTPost::setParent()'s algorithm to detect bad moves (parent in different topic) [11:23:14] yeah? [11:23:22] If a post is moved from topic A to B, and its parent is slated (but not committed) to be moved from A to C [11:23:49] oh yes, that's a loophole. [11:23:51] They'll end up being in different topics, but the array_intersect() call will return array( A ) [11:24:21] It's really just a sanity check, unless you can think of an easily implementable way to detect that case [11:24:27] I really don't want it to get in the way [11:25:17] What exactly is the requirement, anyway [11:25:27] for what? [11:25:32] The bad move detection? [11:25:37] Well, when the parent is slated to be moved, it gets weird [11:26:01] The point in that is not to get in the way of having a few consecutive operations that get committed in a funny order [11:26:22] I mean, I wouldn't be averse to saying just "Sorry, you have to move the topic, THEN you can change the parent" [11:26:30] but i feel like it might cause pain and annoyance [11:26:38] though arguable corrupt databases are more painful and annoying [11:26:44] If you only allow the child's move if the parent is slated to be in the child's prospective (slated) topic, then things get weird when the parent is reset() [11:27:27] yes [11:27:56] the point is that it's a sanity check, not an exhaustive validity check [11:28:04] though like I say, maybe it makes sense to turn it into the latter [11:29:10] OK [11:29:24] Well there's gonna be ways around it always, I guess [11:29:43] If you want to make it an airtight validity check, it'd have to be upon commit anyway [11:29:45] yeah, though if we're designing for robustness [11:29:48] ... [11:30:16] Slackness with robustness is why it's being rewritten in the first place [11:30:22] Yeah [11:30:47] So my suggestion is you check the validity upon commit, otherwise loopholes will continue to exist [11:31:21] It also means you can forget about checking your own past state, because you've already decided you're gonna commit the prospective future state [11:31:49] And for friend objects you should look only at the past (current) state cause that's what's in the DB right now [11:32:07] nod [11:32:20] So that simplifies the check to $child->getPendingTopic() == $parent->getCurrentTopic() [11:37:27] Hmm, the $fetchText issue occurs in PostVersion as well [11:37:39] Probably worth checking all classes for $fetchText and lack of inheritance [11:38:23] yep [11:47:55] Shouldn't PostVersion.php:138 use <= instead of < ? [11:48:30] mmm, probably [11:48:35] Also, empty() usage [11:48:40] (elsewhere) [11:48:55] where? [11:49:05] initialiseFromRow() [11:49:14] In PostVersion, and IIRC in other classes as well [11:49:18] it's a sensible usage [11:49:22] because the rows might not have those fields [11:50:03] Sure [11:50:20] But Tim hates empty() because of its error-suppressing behavior [11:50:36] So the appropriate thing is probably is_null() [11:51:55] no, it would be isset() && !is_null() [11:52:16] because the person who created the row may or may not have joined on the appropriate table [11:52:19] Oh, right [11:55:36] Hmm, isMutable() looks like something that could be abstracted into the object/version classes [11:55:57] yep [12:01:04] TopicVersion.php:132 has a similar thing with < vs. <= [12:02:19] Also, some setters don't check mutability [12:02:59] Alright, all of this stuff looks good [12:03:12] It could be refactored a bit but you were already aware of thar [12:03:13] *t [12:05:52] yeah [12:06:13] great, is that it? [12:06:39] Yeah, I've read through all of it [12:08:28] excellent! [12:08:33] thanks so much for looking over it all [12:08:37] you picked up a few things that testing didn't [12:13:02] Testing software? [12:13:03] Wassat? [12:13:08] *Reedy stares blankly at werdna [12:13:11] haha [12:13:15] You're welcome, werdna [12:13:18] Now I should grab some food [12:13:57] <^demon> Testing's for squares. [12:14:18] Do you get strict errors? Yes; commit anyway [12:15:29] <^demon> I really need to fix our pre-commit lint test. [12:15:58] Plox. [12:17:12] <^demon> What baffles me is /nothing/ changed. [12:17:16] <^demon> But it just stopped working [12:23:09] :D [13:13:21] hey RoanKattouw, I'm integrating with WikiEditor, and I'm adding a button using 'button-msg-label' : function() { /* onclick */ } [13:13:54] I see that in _createButtons() it rewrites that to 'button-msg-label' : { 'click' : function() { /* onclick */ } } [13:14:04] and then runs .attr( props, true ) [13:14:32] I've suddenly found (possibly MW 1.16.0 --> 1.16.4 related) that it *calls* the function [13:14:44] and errors out because the function calls .data('context') [13:15:02] should that happen when running .attr()? [13:16:13] Hmm this sounds bad, lemme have a look [13:16:37] Oh, that'd be the new JUI version then I guess [13:16:44] bah [13:16:48] .attr() shouldn [13:16:54] 't call your function [13:17:16] I think I've worked around it with this: var context = $j(this).data('context'); [13:17:20] if ( typeof context == undefined || !context ) { [13:17:23] return; [13:17:25] } [13:17:31] Can you figure out where the call comes from (call stack / backtrace) using Firebug? [13:18:24] yeah [13:18:29] I'll just need to kill my workaround [13:18:33] Nah [13:18:54] Just set a breakpoint on that return; [13:18:54] And run it in debug mode so it's on its own line [13:21:54] hold on, just figuring out how to copy-paste [13:22:20] Ctrl + C, CTRL + P [13:22:28] thanks Reedy [13:22:37] np [13:23:38] weird, it's coming from skins/common [13:23:43] it's only a 1.16.4 wiki [13:23:47] oh right, $wgIncludeJQuery [13:24:24] how do I stop it from sending that minified? [13:25:19] I can't, I have to hack the source [13:25:23] Hah [13:25:37] mv jquery.min.js jquery.min.js.bak; cp jquery.js jquery.min.js; [13:26:32] it's in jquery.extend.prop() [13:26:32] if ( jQuery.isFunction( value ) ) [13:26:33] value = value.call( elem, i ); [13:26:40] well that's rather straightforward [13:27:04] OK, and where does that get called from? [13:27:27] um, jquery.fn.attr [13:27:28] Maybe you could pastebin the complete call stack? [13:28:02] it's quite simple, actually [13:28:04] I'm especially curious what the arguments to attr end up being [13:28:12] if ( jQuery.isFunction( value ) ) [13:28:17] oops [13:28:25] no idea how to get a full call stack copy from chrome [13:29:29] .attr() is called with { 'click' : function() {/*my func*/}, 'text' : 'my text' } [13:29:36] as the first arg [13:29:36] Yeah that's bad [13:29:38] and true as the second [13:29:54] well, this is what causes it [13:29:55] The second arg will be ignored because the first is an object [13:29:58] props = $.isFunction( props ) ? [13:29:58] { click: props, text: name } : [13:29:59] props; [13:30:12] Oooh [13:30:16] I think I understand [13:30:30] This JUI version is probably only compatible with jQ 1.4+ [13:30:39] And you're probably using jQ 1.3.2 [13:30:52] * jQuery JavaScript Library v1.3.2 [13:30:53] * http://jquery.com/ [13:31:06] Try upgrading jQuery in-place to 1.4.4 [13:31:11] Or whatever the latest version is [13:31:33] ah that will be the patch I forgot to appl [13:31:36] You've somehow managed to use a too-new version of jQuery UI, so... :) [13:34:40] ok, that's fixed [13:34:40] thanks [13:34:54] now to try and make this wiki run at a faster than glacial pace [13:35:19] too much SMW :( [13:37:59] <^demon> !hss [13:37:59] --elephant-- I don't know anything about "hss". [13:38:04] <^demon> Wha? [13:40:45] different b [13:40:48] brain [14:06:17] actually it looks like wfSetupSession() is what's making it slow [14:07:15] well, maybe [14:07:15] query: COMMIT76.82% [17:09:41] awjr: RoanKattouw: TrevorParscal: howief: alolita [17:23:44] RoanKattouw: Krinkle: awjr: howief: howdy! [17:26:15] hey [17:27:50] TrevorParscal: I merged those revs to 1.17wmf1, you can svn up on prototype and they'll appear [17:28:52] k [17:30:30] I updated it, and verified it's coming from http://svn.wikimedia.org/svnroot/mediawiki/branches/wmf/1.17wmf1 but I didn't see the change come through... [17:30:34] lag? [17:30:45] TrevorParscal, RoanKattouw, Krinkle - i'm not seeing the article feedback tool on the ArticleFeedbackTest article on prototype [17:30:59] that's because of this patch [17:31:07] TypeError: Result of expression 'mw.user.bucket' [undefined] is not a function. [17:31:11] there's a js error because the bucket thing I added to core needs merging [17:31:20] ahha [17:31:23] svn: Failed to add directory 'extensions/EmailCapture': object of the same name already exists [17:31:35] what's with that? [17:31:47] we could just remove it and update again I guess [17:31:50] someone added it manualluy [17:31:54] theeere we go [17:32:04] ok [17:32:07] caching issue [17:32:10] svn up just updated a few dozen files [17:32:10] that was likely my bad, sorry [17:32:15] i get the bucket code now [17:32:59] wikimedia button image is still missing... pdhanda has been moving things around [17:33:02] whatever [17:33:16] I have? [17:33:29] *pdhanda looks [17:33:42] Wikimedia button image? [17:33:48] If it's not AFT-related we shouldn't care right now [17:34:01] k, good :) [17:34:49] testing email [17:35:04] it came into my junk mail :( [17:35:09] hotmail doesn't like Wikipedia [17:35:59] i don't think the final email text got used [17:36:34] the email field is showing up after submission -- it should not appear once an email is submitted [17:37:28] Krinkle: could you look at that? [17:37:38] I'm working on making sure the email content is correct [17:38:29] howief: You reported this before and it worked fine when we tested, can you verify once more please ? [17:38:38] yes [17:38:46] so i think this is what is happening [17:39:02] upon submission of invalid email address, the email address field reappears [17:39:16] upon submission of valid email address, the email address field does not reappear [17:39:21] let me see if i can verify this [17:39:32] BTW can someone tell me how many total CTA's have been reported? [17:39:45] Ahm [17:40:58] howie, normally it's frowned upon to put project specific content in an i18n file [17:41:14] we have content in this text like "Wikipedia, the encyclopedia anyone can edit" [17:41:15] howief: Yes, although I don't remember writing it that way, it does reflect what I see. [17:41:27] Do we want that otherwise ? [17:41:29] howief: [17:41:31] | ext.articleFeedback@1-pitch-survey-show | 2925 | [17:41:33] | ext.articleFeedback@1-pitch-edit-show | 2892 | [17:41:35] | ext.articleFeedback@1-pitch-join-show | 4955 | [17:41:40] woah [17:41:44] that's production? [17:41:55] awjr, btw, I feel your PayPal code pain. We've got to use it for a crappy piece of coursework, and it's instructions are often wrong [17:42:05] Yeah that's enwiki [17:42:18] It's since the last AFT deployment on, what was it, April 17? 19? [17:42:20] Reedy: you can almost assume whatever you need to know is not documented and requires reverse engineering [17:42:29] or if it is documented, it's incorrect :p [17:42:35] :D [17:42:41] ( Please save this file into the AppCode sub directory of your .Net web app ) [17:42:42] or if you need to talk to anyone at PayPal for support, you're better off putting your head in an oven [17:42:44] I have no AppCode [17:42:45] TrevorParscal: Emailcapture field properly hides (both within the same load and after refresh / other page views), *if* a valid email adres was entered and saved by the server. [17:42:48] but i have an App_Data folder [17:43:04] RoanKattouw: that's interesting because the numbers are orders of magnitude greater than the last toolserver dump [17:43:13] If the option was not checked, mail invalid or server did not save, it doesn't set the hide cookie / nor does it do the $.fn.hide() call [17:43:14] toolserver is full of lies [17:43:45] howief: I can generate an out-of-schedule dump for you, that'll take approx 20 mins. Want one? [17:43:51] RoanKattouw: What're those numbers in the second column you pasted ? [17:43:53] Krinkle: that's not ideal, but it might be okay [17:44:03] Krinkle: Number of hits for each clicktracking key [17:44:07] k [17:44:12] howief: if I'm allowed to drop ", the encyclopedia anyone can edit" from the email text, we have no problems - it will still say wikipedia [17:44:21] howief: It's pretty easy to adjust, how would you prefer it to be ? [17:44:23] {{SITENAME}} [17:44:28] RoanKattouw: right [17:44:38] Reedy: you have to do paypal stuff for a class you're taking? [17:44:45] Krinkle: yes, let's change because there's a funky interaction thing that happens [17:44:49] but "{{SITENAME}}, the encyclopedia anyone can edit" is pretty FAIL [17:44:50] with the current implementation [17:45:14] Even for Wikimedia projects other than wikipedia. [17:45:29] TrevorParscal: that works [17:45:32] ok [17:45:39] awjr, it's half of a piece of coursework, which is 30% of the module, and another piece makes up another 30% [17:45:59] oof [17:46:00] RoanKattouw: the out-of-schedule dump would be awesome, but only afte we're done with deployment :) [17:46:11] OK [17:46:23] TrevorParscal: Should we increment the version number for this deploy? Any reason to do so? [17:46:47] the cookie names for bucketing are going to change anyways [17:46:51] I think we should [17:47:01] howief: do we need to support all platforms/browsers in that matrix? [17:47:03] to avoid known, and unknown issues with not [17:47:49] awjr: i wish i could give you a solid answer [17:48:01] but we don't have a test matrix we've all agreed on [17:48:06] heh ok [17:48:13] so in the matrix, i've greyed out the ones that I don't think need testing [17:48:19] RoanKattouw: suggestions on how to do http://wikimediafoundation.org/wiki/Privacy_policy properly in an i18n file? [17:48:24] perhaps we should hammer that out for the next deploy so we're surely only focussing on the stuff we care about [17:48:30] yes! [17:48:38] it needs to render to a full URL, because it's being sent to someone via email [17:48:42] and there may be a diff test matrix for major deployments vs. minor ones [17:48:48] [[Privacy policy]] ? [17:48:57] TrevorParscal: Just put it in, worry about properly later [17:49:03] We're supposed to deploy real soon now [17:49:19] arg [17:49:26] Also, wg var [17:51:08] whats the wg var for "http://domain.org/wiki/" ? [17:51:24] can someone please test on windows? [17:51:27] wgArticlePath [17:51:29] $wgArticlePath? [17:51:30] ok [17:51:33] Contains $1 so you have to .replace() that [17:51:41] TrevorParscal: Can I get that wg var to disable the special page like now? [17:51:49] yes [17:51:50] howief: What do you need me to do? [17:51:57] TrevorParscal: Those links should probably go in WikimediaMessages [17:52:06] Eventually they should, yes [17:52:06] core default to the link in the footer [17:52:11] submit a few ratings and emails on IE please [17:52:13] (privacy) [17:52:25] Oh wait, this is not code we're deploying now, it's dashboard, so ignore my "screw properly" comment :) [17:52:32] extension default* [17:52:36] howief: OK, on it [17:52:44] thanks [17:53:19] lol, I'm now running FF4, reKonq and IE in a VM [17:53:31] {{int:privacypage}} privacypage [17:53:35] Project:Privacy policy [17:53:36] BTW, I just rated -> submitted valid email -> received edit CTA --> successfully saved edit [17:53:41] is that path showing up on click-tracking? [17:53:56] TrevorParscal: I did privacy link already [17:54:07] it's in the -note below the helpimprove-email field [17:55:17] k [17:55:41]