[17:21:42] So, apparently there are executive team updates on-wiki every week [17:21:42] https://office.wikimedia.org/wiki/Leadership/Executive_Team_Weekly_Updates [17:21:45] Didn't know about these! [21:03:20] #startmeeting RFC meeting [21:03:21] Meeting started Wed Jun 7 21:03:20 2017 UTC and is due to finish in 60 minutes. The chair is TimStarling. Information about MeetBot at http://wiki.debian.org/MeetBot. [21:03:21] Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. [21:03:21] The meeting name has been set to 'rfc_meeting' [21:03:30] #topic RFC: Drop requirement to define a talk namespace for every subject namespace [21:04:00] hi everyone! [21:04:12] #link https://phabricator.wikimedia.org/T165149 [21:04:30] I didn't write a reminder email today. Let's see... [21:04:49] anyone here to talk about dropping the reuirement for talk namespace? [21:05:29] hm, exciting... [21:05:36] if nobody cares, we should just drop it :) [21:05:37] canTalk()? [21:05:51] interesting choice of verb [21:06:08] :) [21:06:31] I would have said hasTalk() [21:06:39] TimStarling: blame Rob Curch ;) [21:06:48] * Church [21:07:00] right [21:07:10] "Can this namespace ever have a talk namespace?" [21:07:46] kind of odd to abbreviate that sentence to "canTalk" [21:07:51] yea. we already have this function, and not all namespaces can. [21:08:14] But all with positive IDs are expected to have (or be) a talk namespace [21:08:52] well, we have canExist... [21:09:04] canExist makes grammatical sense [21:09:23] so the problem is just that things are not calling canTalk()? [21:09:59] yes. and that getPrefixedText() does not handle bad namespace IDs gracefully [21:10:25] it will just prepend the namespace name it gets from getNsText [21:10:37] which will be false if the namespace doesn't exit [21:10:46] so you end up with ":Foo". [21:10:58] which of course points to NS 0 [21:11:08] TimStarling: so, seems like it's just the two of us. [21:11:12] some functions should throw exceptions if the namespace is nonexistent [21:11:20] like Title::makeTitle() [21:11:36] if you use a non-existent integer ID, it should throw an exception [21:11:59] currently, it doesn't even throw an exception if the title is empty. [21:12:15] but i agree. [21:12:23] do you think it should also throw on malformed titles? [21:12:45] huh, we have newFromTextThrow, the throwing version of newFromText... [21:13:06] right, Title::newFromText() is for user input, it doesn't need to throw [21:13:15] but integer IDs are assumed to not come from the user [21:13:40] TimStarling: so you think it should not be possible to construct a Title object with an invalid namespace? or invalid text? [21:14:07] i'm wondering whether this may cause issues in some cases, e.g. when a namespace has been removed from the config (extensio removed) [21:14:14] currently Title::newFromText() will return false on invalid text, won't it? [21:14:23] I think so [21:14:47] I'm wondering what happens if you remove a namespace, and then try to look at the log, or recentchanges [21:15:05] it would blow up, if we didn't add a lot of catch clauses, right? [21:15:05] the traditional distinction between returning false and throwing an exception is that throwing an exception is for developer errors, whereas returning false is for user errors [21:15:38] i tend to use exceptions for user errors, too, and make the distinction based on the type of exception [21:15:54] but it's hard to introduce this into legacy code without breaking lots of things [21:16:26] when I first introduced exceptions into MW I started a discussion along these lines on wikitech-l [21:16:39] > what happens if you remove a namespace, and then try to look at the log --> There are links that start with a colon (eg [[:Title]]) IIRC [21:17:03] I found the 'canTalk' name kind of cute :-) but yes, hasTalk is more accurate .. but i don't care really. [21:17:11] Vulpix: right now, yes. But if we go with tim's suggestion, there wouldn't be. [21:17:16] pages don't talk, people talk [21:17:20] oh, hey, people! [21:17:34] Daniel should rename those functions and deprecate the old names [21:17:44] TimStarling, ya ya ... :) [21:18:38] I agree that it should not be possible to construct Title objects with a bad namespace, but I fear it will break a lot oef existing code [21:18:59] a compromize would be to allow them to be created, and only fail when trying to do specific things with them [21:19:19] getPrefixedText could throw - or return "", or return a link to Special:BadTitle. [21:19:43] How would namespaces without talk pages have gerneral discussion rather than on the content page [21:19:54] getTalkPage could throw, or return an invalid Title. [21:20:31] Zppix: they wouldn't. that's the point: allow extensions to prevent that. One example where this makes sense are Flow Topic pages. they *are* discussions. [21:20:53] And flow already disables the talk namespace. doesn't define it, and has some hacks to deal with the fallout [21:21:16] But according to the documentation, we require the talk namespace to be defined. and according to the code, strange things can happen if you don't. [21:21:28] So, question one: [21:21:43] would it be possibly for talk pages to be added for ns without talk pages if a community desires? [21:21:49] should we drop the requirement? if we want to do that, what unindended consequences could that have? if we don't, why not? [21:22:18] it looks to me that since flow topics don't have talk NS, it's effectively already dropped? [21:22:34] SMalyshev: violated, not dropped [21:22:41] semantics :) [21:22:51] pages in nonexistent namespaces should probably be treated as nonexistent [21:22:54] SMalyshev: the requirement was never really clear. it's an undocumented assumption in the code, and documented only in an example for extension config. [21:23:22] I'd say if we have cases where NSes should exist without talk, this should be supported [21:23:23] TimStarling: pages, yes. does that mean we can't render their title? [21:23:30] in recentchanges, for example= [21:23:38] we probably shouldn't try to link to them [21:23:45] but we'd want to show... something, i guess? [21:23:58] {{TALKPAGENAME:Topic:AA}} --> :AA [21:24:00] If the requirement is dropped, there should be: a way for communities to request that ns have a talk page enabled, default to the ns to have one unless defined otherwise [21:24:03] so the Title object would need to exist. Or at least some LinkTarget object [21:24:28] Vulpix: yes. which may actually exist, and cause confusion. Or not exist, and get created in the main NS [21:24:46] hmm, nonexistent does get a bit tricky with query limits [21:24:57] TimStarling: what do you mean? [21:25:33] if you want 50 pages, and 10 are bad titles, do you then return 40 pages? or do a second query to populate the remaining 10 slots? [21:25:43] in a hypothetical batch API [21:25:46] can we make getTalkPage() return Special:ThisNamespaceHasNoTalkPages or something like that? [21:26:01] TimStarling: oh, you want to skip any log line or rc entry etc that refers to a page in a non-existing namespace? [21:26:02] it exists already SMalyshev [21:26:05] i wouldn't want to do that. [21:26:15] SMalyshev: Special:badTitle [21:26:17] SMalyshev: it's just not called [21:26:43] we could do that. but Tim mentioned on the ticket that this may do more harm than good. [21:26:46] so why the NS problem then? [21:26:49] DanielK_WMDE_: yeah, your idea is probably better [21:26:53] it would provide maximum backwards compat. [21:26:58] Bad title is vague i would suggest another error page/info be created for the ns talk [21:27:10] except that then, all formatters have to know how to display invalid titles [21:27:16] unless you return Special:Badtitle [21:27:45] yes. that's why i suggested to return Special:BadTitle :) [21:27:52] but i agree that it's not very nice. [21:28:09] i do think though that it must be possible to construct a Title (or at least, a LinkTarget) for an invalid namespaces [21:28:15] so we can still look at logs [21:28:26] user contribs, rc, watchlist... [21:28:54] that sounds a bit dangerous, as code makes all kinds of assumptions about what titles do, and bad titles may not hold these [21:29:42] like that empty content debacle we had... I am afraid we'll get into the same thing with bad titles [21:30:03] if you want the logs to actually make sense, it shouldn't just be Special:Badtitle, it should be something unique [21:30:21] Special:BadTitle/NS77:Foobar [21:30:32] yes [21:30:44] most usages of BadTitle are already like this [21:30:58] they encode *something* after the slash [21:31:50] so, let's assume getNamespaceText() returns "Special:BadTitle/NS77" if 77 is not a valid namespace [21:32:20] :P [21:32:22] that would cause getPrefixedText to just work (tm) [21:32:40] that breaks a lot of assumptions [21:32:50] what should we do with getTalkPage()? Should it construct a bad Title object, or a Title object for Special:BadTitle? [21:33:06] TimStarling: what assumption does it break? [21:33:35] being able to round-trip from the text form [21:33:49] yes, that would break. [21:33:54] not sure that's a bad thing in this case [21:34:01] the namespace of Special:BadTitle/NS77:Foo is Special, isn't it? [21:34:09] yes [21:34:36] DanielK_WMDE_: by getNamespaceText do you mean getNsText()? [21:34:54] to allow roundtrip parsing, we could do Special:BadNamespace/77:Foo. TitleParser could know how to construct the correct bad title from it [21:34:56] seems like unnecessary complexity, introducing namespace names with colons in them that magically concatenate to make special page titles [21:34:59] SMalyshev: yes, sorry [21:35:38] hmm then it looks weird that getNsText returns weird NS name that includes full page name [21:35:43] shouldn't it just return Special? [21:36:18] SMalyshev: that's what tim is complaining about :) [21:36:21] TimStarling: it's surprising, and not very clean. but it doesn't seem complex - in fact, it seems like the simplest solution that will break the smallest number of things. [21:36:23] public function getOtherPage() { [21:36:23] if ( $this->isSpecialPage() ) { [21:36:23] throw new MWException( 'Special pages cannot have other pages' ); [21:36:23] } [21:36:39] the downside is that we are hiding and error, which is generally not a good thing. [21:36:41] so there is a precedent for making getTalkPage() throw [21:36:55] but is it realy and error? [21:37:44] yes, the caller should call hasTalk() first [21:37:47] if we have canTalk and it produced false then kind of yes... but the question is how much code would be broken by this. [21:37:54] i.e. canTalk() after it is renamed [21:38:02] if the answer is "a lot" then we may want to compromise on it [21:38:04] TimStarling: i'm ok with letting getTalkPage throw. There are not that many places in the code that call it on something other than the user page. [21:38:50] SMalyshev: not much in core, or the handfull of extensions i looked at [21:38:58] but given getOtherPage already throws, I think it's ok [21:39:06] as for makeTitle(), maybe there should be a throwing variant, like for newFromText [21:39:16] lots of calls, nearly all of them are just trying to get the user talk page. which should always be there [21:39:31] oh, right, yes: all (positive) default namespaces should always have talk pages. [21:39:46] TimStarling: i like that idea [21:40:20] TimStarling: so how about getPrefixedText on a Title with a bad NS, from some extension that got removed? [21:40:46] it currently produces a Title that will refer to the main namespace when turned into text [21:40:55] it already breaks the round trip [21:41:43] makeTitle() with an invalid namespace could generate a Title object with mNamespace being the invalid namespace [21:41:45] maybe we can make getTalkPage throw, and have makeTitleThrow, and still have getPrefixedText produce Special:badTitle/something? [21:42:03] yes [21:42:17] #info maybe we can make getTalkPage throw, and have makeTitleThrow, and still have getPrefixedText produce Special:badTitle/something? [21:42:31] That seems to be a good compromize in terms of safety and interoperability [21:43:18] it is actually spelt compromise, even in US english [21:43:23] otherwise yes :) [21:43:36] oh, whateverz ;) [21:44:19] we could have a flag in makeTitle, instead of makeTitleThrow. And have it throw per default. [21:44:31] but that probably would break too many things... [21:44:59] makeTitle doesn't throw currently does it? [21:45:02] TimStarling: how about makeTitleSafe vs makeTitle? Do we need a throwing variant of both? [21:45:04] Isn't that why we have makeTitleSafe already? [21:45:18] well, it would require query result formatters to be aware of invalid titles [21:45:26] makeTitle is for "I know what I'm doing, this data came from the DB and is guaranteed to be valid" [21:45:37] yeah, except it's not guaranteed to be valid [21:45:46] as discussed above [21:45:47] RoanKattouw: but it's not. namespaces get removed. [21:45:48] AIUI you can already makeTitle() a Title with an invalid namespace, and it just renders as ":Title" [21:46:05] RoanKattouw: that's the problem we are trying to solve. it shouldn't. [21:46:13] so the question is: what should it do instead? [21:46:22] makeTitleSafe already has an error handling convention [21:46:28] because it's assumed to come from user input [21:46:33] Yeah makeTitleSafe can just return null [21:46:33] } catch ( MalformedTitleException $ex ) { [21:46:34] return null; [21:46:45] so there's no need for variants [21:46:46] it should do that for a bad namespace, too [21:46:51] As for makeTitle, I argue that it should be OK for it to generate awkward/bad Title objects [21:46:51] yea, i see [21:46:54] it already does [21:46:58] if ( !MWNamespace::exists( $ns ) ) { [21:46:59] return null; [21:47:11] oh, right! [21:47:22] RoanKattouw: yes. but what should those render as? [21:47:35] RoanKattouw: current proposal: Special:BadTitle/NS77:Foobar [21:47:59] Right now it's :Foobar which I agree is suboptimal [21:48:00] or Special:BadNamespace/77:Foobar, which would allow rond trips... kind of. [21:48:35] we are getting close to the ten minute warning [21:48:38] I guess the latter would round-trip if we special-cased it, yes [21:48:53] Special:BadTitle/NS77:Foobar round trips to Special:BadTitle/NS77:Foobar [21:49:02] but with ns=-1 in the latter case instead of 77 [21:49:08] TimStarling: Yes but does its ->getNamespace() round-trip [21:49:08] byond the details - any objections to dropping the requirement to define a talk namespace? turning the MUSt into SHOULD? [21:49:10] Exactly [21:49:15] I think that's OK [21:49:28] any objections to making getTalkPage throw if there isn't a talk namespace? [21:49:30] sounds good [21:49:37] on both [21:49:42] DanielK_WMDE_: I'm in favor of the general idea, but I'm responsible for Flow, so... :D [21:49:54] hehe :) [21:50:06] I stand to gain from this, given that Flow is one of the current use cases of a missing talk namespace (2601) [21:50:23] excellent :) [21:50:54] TimStarling: so, BadTitle? or BadNamespace, with fake round-trip support? [21:50:59] is there some sort of validity check, like Title::isValid()? [21:51:07] to validate the result of Title::makeTitle()? [21:51:17] afaik, all titles are assumed to be valid [21:51:26] we could add hasValidNamespace, maybe? [21:51:51] we have isValidredirectTarget [21:51:57] we could have isValidPageTitle [21:52:05] namespaceExists()? [21:52:09] I don't think there is a validity check and I don't think that was in the spirit of the original intent of makeTitle() [21:52:20] The intent was to bypass validity checks because you *knew* it was valid [21:52:23] TimStarling: yes, but that doesn't belong to the title object, i think [21:52:24] we're talking about an invalid title with a nonexistent namespace [21:52:29] not an invalid namespace [21:52:37] Then again, we could take all the checks/normalization that makeTitleSafe does, and factor those out into isValid? [21:53:02] RoanKattouw: no, i think early validation is the right thing in many situations [21:53:09] it's just that we don't want it in all situations [21:53:39] log entries for pages in namespaces that are no longer defined should still be redable, and we should still be able to render them, with not-totally-misleading links [21:53:42] hm... [21:53:57] so if a namespace was removed, so we stil lwant to be able to load its content? [21:54:00] I'd think so [21:54:16] WikiPage::factory should work with a Title that has a bad namespace [21:54:25] but page creation should fail [21:54:28] scope creep [21:54:31] oh, fun details to figure out later :) [21:54:34] yea [21:55:08] TimStarling: but that would actually be a very good reason to have isValid [21:55:20] right [21:55:59] so, 5 minutes left. [21:56:09] we seemed to have covered the main questions, and then some [21:56:16] anything we missed? [21:56:30] impact on 3rd party tools if we allow pages with no talk page? [21:56:48] pywikibot? dump scripts?... [21:58:34] api.php will presumably also return Special:Badtitle? [21:58:52] I'd think so [21:58:57] it'll do what Title does [21:58:58] so the impact will be the same as for humans [21:59:13] they'll get a stupid broken link, click it and get confused at the result [21:59:18] but probably not fatally confused [21:59:30] still better than linking to the main namespace [21:59:37] right [22:00:08] but for the api, we may be able to omit such links, in some situations. when there is no talk namespace, and the query asks for associated talk pages, for example [22:00:27] but we shouldn't omit bad titles from listings. we could perhaps mark them, if the format allows this [22:00:42] #idea mark bad titles in api output [22:00:59] #idea allow loading, but not editing, of pages in namespaces that no longer exist [22:01:06] ok. done? [22:01:15] #endmeeting [22:01:15] Meeting ended Wed Jun 7 22:01:15 2017 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) [22:01:15] Minutes: https://tools.wmflabs.org/meetbot/wikimedia-office/2017/wikimedia-office.2017-06-07-21.03.html [22:01:15] Minutes (text): https://tools.wmflabs.org/meetbot/wikimedia-office/2017/wikimedia-office.2017-06-07-21.03.txt [22:01:15] Minutes (wiki): https://tools.wmflabs.org/meetbot/wikimedia-office/2017/wikimedia-office.2017-06-07-21.03.wiki [22:01:16] Log: https://tools.wmflabs.org/meetbot/wikimedia-office/2017/wikimedia-office.2017-06-07-21.03.log.html [22:01:22] thanks everyone! [22:01:52] thanks especially to Tim, for all the input, and for chairing!