[22:02:00] RFC discussion running a few minutes late, bare with us... [22:03:39] o/ [22:04:12] hey legoktm! [22:04:19] let's see if i remember how to use meetbot [22:04:27] #startmeeting [22:04:27] DanielK_WMDE_: Error: A meeting name is required, e.g., '#startmeeting Marketing Committee' [22:04:33] hehe [22:04:48] #startmeeting ArchCom office hour: Deprecation policy for PHP code in MediaWiki [22:04:48] Meeting started Wed Dec 14 22:04:48 2016 UTC and is due to finish in 60 minutes. The chair is DanielK_WMDE_. Information about MeetBot at http://wiki.debian.org/MeetBot. [22:04:48] Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. [22:04:49] The meeting name has been set to 'archcom_office_hour__deprecation_policy_for_php_code_in_mediawiki' [22:04:54] #topic Deprecation policy for PHP code in MediaWiki [22:04:59] #link https://phabricator.wikimedia.org/T146965 [22:05:30] #link https://phabricator.wikimedia.org/E412 [22:06:10] oh, the meat of the proposal is really on mediawiki.org [22:06:11] #link https://www.mediawiki.org/wiki/Requests_for_comment/Deprecation_policy [22:06:18] ok... [22:06:30] legoktm: so, what are the main questions you want to get answered today? [22:06:59] Do people think this is reasonable? What's missing? When can we implement this? :) [22:08:18] i didn't get around to re-reading this (shame on me). but one of the big questions is always: [22:08:33] what do we do with (semi)unmaintained extensions? [22:08:41] seems reasonable to me [22:09:02] I think it would be nice to address https://www.mediawiki.org/w/index.php?title=Topic:Tfxuk4z5er4jw6qk&topic_showPostId=th2zprlonf8kwagw#flow-post-th2zprlonf8kwagw [22:09:13] but that's more of a documentation issue than a policy issue [22:09:35] I suspect we would want to tag a *lot* of things @unstable [22:09:47] I would restrict the PHP API to which it applies to that which appeared on at least one official release [22:10:31] when something gets merged, we expect it will be stable, but could be changed later [22:10:45] and makes little sense to follow all those steps for something that was never released [22:10:47] "Any relevant documentation in the git repository or on mediawiki.org MUST be updated once the change is approved." <-- it's hard to even find all relevant documentation [22:11:41] "Developers SHOULD update any extension or skin bundled with the MediaWiki tarball" <-- they should also update all usages in core, right? Does it say this somewhere? [22:12:17] DanielK_WMDE_: If an extension is unmaintained, and there's no one to pick it up, then it should be archived or documented as broken for future releases [22:12:22] Platonides: yes, true; though that is going to bit things that are tied closely to the wmf deployment cycle, like Wikibase [22:12:42] legoktm: yes, but nobody takes care of doing this [22:12:52] DanielK_WMDE_: that's soft-deprecation. Updating stuff in core MUST be required for hard deprecation. [22:13:11] e.g. with the Linker::link() deprecation, it's currently soft-deprecated but there are still calls in core to it [22:13:40] DanielK_WMDE_: I wouldn't consider the wmf deployment a release for these effects [22:13:49] DanielK_WMDE_: right, because the current expectation is that core developers are supposed to fix it when they deprecate/remove stuff. This policy would remove that burden from core developers [22:14:27] "developers SHOULD consider usage statistics in extensions." <--- what does that mean? [22:14:41] Platonides: yes, that's a good point. Stuff in master can keep changing without issue until it is released in a stable version. I'll change that. [22:14:43] there should be a way to sign extensions as "I have reviewed it for compatibility with MW 1.XX " and everything not having + not being deployed on some large close-to-master wiki should be assumed unmaintained/broken [22:14:58] but again that's outside the RfC scope [22:15:18] DanielK_WMDE_: If a function is used in every extension, developers shouldn't remove it after the minimum # of releases [22:15:29] legoktm: yes, core MUST be updated before hard deprecation. But I think it also SHOULD be updated upon soft deprecation. [22:15:31] basically to be realistic in how code is being used [22:16:25] Platonides: https://www.mediawiki.org/w/index.php?title=Requests_for_comment%2FDeprecation_policy&type=revision&diff=2313448&oldid=2302204 does that look good? [22:16:33] i very much like the idea of removing the burden of updating all extensions. [22:16:47] that's a major blocker to refactoring our code base [22:17:01] legoktm: LGTM :) [22:17:32] DanielK_WMDE_: updated https://www.mediawiki.org/w/index.php?title=Requests_for_comment%2FDeprecation_policy&type=revision&diff=2313449&oldid=2313448 [22:18:04] I like what I see overall [22:18:24] it's soft-deprecate > remove calls > hard-deprecate [22:18:33] "In some cases, it is necessary to remove code without deprecating it" <-- like, when? [22:18:43] step 2 is not neccessarily plausible to do immediately or in a single step [22:18:44] security issues [22:18:53] legoktm: thanks! [22:19:32] DanielK_WMDE_: security issues, or like the giant SessionManager/AuthManager refactor which removed some code, but also provided a back-compat layer for some things [22:19:40] tgr: the SHOULD line doesn't say it has to be done before the soft deprecationn. [22:19:47] I wonder if there should be an exception for removing methods that were never called by any extension [22:19:48] but if you deprecate, you should do it sooner or later [22:20:15] TimStarling: a "no known callers" rule? [22:20:31] TimStarling: how do we know that though? that's really a "no known callers in Gerrit or Github" which isn't "any extension ever" [22:20:42] tgr: not sure how you are counting the steps [22:21:31] I think there's a lot of code which is extremely unlikely to have ever been called by an extension, and the only reason it's not marked @unstable is because nobody bothered or thought it was necessary [22:21:36] legoktm: but maybe it would be ok to just consider things in the "well known" repos [22:21:38] I like the definition in terms of number of releases [22:21:54] that provides for some flexibility about the actual timeline [22:22:10] but basically, yes, just grep for it [22:22:19] I did note: "Extension developers are encouraged to mirror their code into Wikimedia's Gerrit/Phabricator/Github to make it easier for core developers to identify usage patterns. Extensions that are open source will be given more consideration than those that core developers cannot see." [22:23:09] but I feel like if we make such an exception, we're just going to get random bug reports that are like "hey! I was using that feature you thought no one was using, and now my stuff is broken" - I'd rather just be consistent [22:23:32] legoktm: btw - is the goal to make this an ArchCom approved official policy? should this go into Last Call mode after the meeting (if no objections remain)? [22:23:57] DanielK_WMDE_: Yes and yes [22:24:04] maybe also worth mentioning that it's developers' responsibility to make their code greppable [22:24:09] like with https://www.mediawiki.org/wiki/Localisation#Using_messages [22:24:35] legoktm: one area that is not completely clear yet is how to deal with extension code calling into morally (but not explicitly) private methods (or messing with private members) [22:25:10] gwicke: why would some code be morally but not explicitly private? [22:25:11] tgr: ugh, i hate that ;) [22:25:19] but it's easier with methods [22:25:24] should "no callers" be applied when making those explicitly private, and should the deprecation procedure apply then? [22:25:52] legoktm: i'm wondering about really old code, where methods and fields don't have access modifiers.Many of these were always assumed private, but we have no guarantee nobody is using them [22:25:53] I think it's fine to get such random bug reports [22:25:54] moving something from public to protected should fall under this policy [22:25:57] legoktm: hooks for example provide access to all kinds of objects, not all of which are consistently marked with public / private [22:26:06] legoktm: even making them private would need a deprecation period/preocess, right? [22:26:33] deprecation policy is about balancing the needs of developers and users [22:26:47] (if I can call extension developers "users" of an interface) [22:27:06] legoktm: oh, right, deprecating hooks is another issue. that should be mentioned [22:27:15] TimStarling: if something is unreasonable to keep supporting and it has no callers, then I think it's fine to get rid of right away. But if it's trivial to keep around, I don't think it should be removed for the sake of removal. [22:27:18] gwicke: yes, sadly [22:27:40] gwicke: if it's not marked right now, it's de facto public. [22:27:43] btw, feel free to use #info and friends to populate the minutes [22:28:18] #info seems reasonable to me [22:28:21] legoktm: another option could be to strike a compromise, for example for those that were documented as private (naming convention, comment) [22:28:26] #info I would restrict the PHP API to which it applies to that which appeared on at least one official release [22:28:36] legoktm: OK [22:28:59] TimStarling: if the https://www.mediawiki.org/wiki/Requests_for_comment/Deprecation_policy#Removal_without_deprecation section were expanded to include "not reasonable" in addition to "not possible", would that satisfy your concerns? [22:29:08] #info I like what I see overall [22:29:26] agreed on applying the full policy to privatizing those members / methods that were not otherwise documented as private, and used as public [22:29:28] #info I wonder if there should be an exception for removing methods that were never called by any extension [22:29:51] #info I think there's a lot of code which is extremely unlikely to have ever been called by an extension, and the only reason it's not marked @unstable is because nobody bothered or thought it was necessary [22:29:56] gwicke: right, I think if a function has @private in the doc block, that means it's private. Regardless of what the code actually says. I'll expand the line that says "Classes and/or functions with public visibility MAY also have @internal or @unstable annotations to indicate they are not stable interfaces that SHOULD NOT be depended upon." a bit [22:30:21] there is also the "mFoo" convention [22:30:31] imho, those are morally private already [22:30:38] #info moving something from public to protected should fall under this policy [22:30:52] legoktm: that would probably be fine, I'm just figuring out what the second dot point would mean in that case [22:31:02] #info a preocedure for deprecating hooks should be mentioned explicitly [22:31:28] TimStarling: all the steps that refer to documenting the removal (release-notes, on-wiki) still need to be followed [22:31:32] #info TimStarling: if something is unreasonable to keep supporting and it has no callers, then I think it's fine to get rid of right away. But if it's trivial to keep around, I don't think it should be removed for the sake of removal. [22:31:34] DanielKWMDE: do you think that procedure should differ from the regular APIs? [22:32:18] legoktm: how do you want to apply the deprecation process to making something private? [22:32:19] so add release notes, announce to wikitech-l and update any on-wiki documentation which refers to it (there is likely to be none in the case I'm imagining) [22:33:00] TimStarling: yes [22:33:24] DanielK_WMDE_: I think it works the same way? Just instead of saying it's being removed, it just becomes private [22:33:27] gwicke: not really, but you don't have a place for @deprecated, instead you should put a warning in docs/hooks.txt. And instead of wfDeprecated, you use the $deprecationVersion parameter in Hooks::run. That shouild be mentioned. [22:33:34] should mediawiki-l be used in some case? [22:33:52] legoktm: using @deprecated? How do you do hard deprecation? [22:33:54] legoktm: ok, sounds good [22:34:13] Platonides: I've always understood wikitech-l to be development, and mediawiki-l for user support. But if people think notices should go there too, that's fine with me [22:34:26] #info procedure for hooks: put a warning in docs/hooks.txt. Use the $deprecationVersion parameter in Hooks::run. [22:34:34] legoktm: does that sound good? --^ [22:34:37] DanielK_WMDE_: wfDeprecated(....) and then when the time comes to make it private, remove the wfDeprecated call and change the visibility [22:34:56] I'm thinking mainly in third parties [22:34:56] just add "@note all properties of this class considered private" or something like that? [22:35:05] legoktm: but wfDeprecated would be triggered by internal calls. internal calls are fine. [22:35:06] DanielK_WMDE_: re: hooks, yes. [22:36:01] DanielK_WMDE_: ah hmm. maybe create a private doWhateverInternal() for the private usage, and have the public one emit the warning and call the internal function. Or something. [22:36:01] legoktm: it's unclear to me how hard deprecation can work for a method moving to private. much less a member. [22:36:33] so, you create a new private function, and deprecate and remove the public one [22:36:36] that makes sense [22:36:43] should go into the policy, then [22:37:04] #info to make something private, create a private doWhateverInternal() for the private usage, and have the public one emit the warning and call the internal function. [22:37:16] DanielK_WMDE_: anything with wfDeprecated should be unreachable from non-deprecated code, whether internal or external [22:37:20] won't work for fields, but perhaps we can deprecate direct field access as a whole... [22:37:31] tgr: indeed, that was the point [22:37:31] "MediaWiki core code that triggers hard deprecation warnings MUST NOT be reachable from non-deprecated core code using non-deprecated configuration settings [22:37:35] DanielKWMDE: makes sense re hook procedure [22:37:35] maybe with some magic method? [22:38:02] at some point, it's too burdensome to ensure that those wfDeprecation warnings will be sent [22:38:24] deprecating global variables is fun, too :) [22:38:25] I personally think that accessing fields named mSomething was always wrong [22:38:38] btw, do we have a page explaining how to log those ? [22:38:53] Platonides: log...deprecation warnings? [22:38:53] gwicke: true, but we moved away from the mFoo convention, so you often can't tell by the name [22:38:59] for fields we should just treat soft deprecation as hard, I don't think it's worth the effort and maintenance burden to try to make it emit warnings [22:39:09] __get is not one of PHP's finer parts [22:39:29] #info for fields we should just treat soft deprecation as hard, I don't think it's worth the effort and maintenance burden to try to make it emit warnings [22:39:32] tgr: +1 [22:39:41] DanielK_WMDE_: if it's gone everywhere already, then yeah that's moot [22:39:46] legoktm: as a simple page that can be linked [22:39:54] gwicke: not everywhere [22:40:32] lets just declare those as implicitly private? [22:40:42] yea. how? [22:40:54] I don't think we can just declare m whatever private [22:40:57] (remaining fields named "mSomething") [22:41:14] legoktm: why not? well, protected, really. [22:41:18] we could mention it in the RFC [22:41:22] I'm pretty sure we have some classes calling internal fields of friend ones [22:41:27] We should look at actual usage before just declaring that [22:41:36] Platonides: yea, but that should be fixed. [22:41:39] or add comments, and then have something about explicit markers in the RFC (if it's not yet there) [22:42:14] "Some legacy code may not have any visibility modifiers, in which case it is not considered to be part of the stable interface. Developers SHOULD add visibility modifiers as soon as possible, and use judgement when making de-facto public code protected or private. New code MUST have explicit visiblity set on all properties and functions." [22:42:17] #info the policy should mention how to mark fields as deprecated, or for becoming private [22:42:28] that covers most of the accidentally public stuff IMO [22:42:38] tgr: +1 [22:42:40] +1 [22:42:50] #info "Some legacy code may not have any visibility modifiers, in which case it is not considered to be part of the stable interface. Developers SHOULD add visibility modifiers as soon as possible, and use judgement when making de-facto public code protected or private. New code MUST have explicit visiblity set on all properties and functions." [22:42:51] default-private is better than default-public [22:42:51] hm [22:42:59] am i overdoing the #info? sorry for the noise. [22:43:04] (it's from the document) [22:43:10] DanielK_WMDE_: Updated for the hooks stuff: https://www.mediawiki.org/w/index.php?title=Requests_for_comment%2FDeprecation_policy&type=revision&diff=2313460&oldid=2313456 [22:43:17] i think it covers this [22:43:27] oh, okay [22:43:42] * gwicke clearly did not read that part carefully enough [22:44:15] legoktm: thanks! [22:44:38] so to clarify, "and use judgement when making de-facto public code protected or private" means "apply the deprecation process only where it is needed"? [22:45:55] it means that if something doesn't have visibility set, you can make it private or protected without applying the process, but to use good judgement when doing so [22:45:59] * DanielK_WMDE_ likes using RevisionSlider to track legoktm's updated to the policy [22:46:30] legoktm: cool, that makes sense to me [22:47:05] do you think it's worth making that slightly more explicit, or am I the only one to fear ambiguity there? [22:48:02] btw - wikidata has a Stable Interface Policy. That's kind of related, perhaps have a look for reference: https://www.wikidata.org/wiki/Wikidata:Stable_Interface_Policy [22:48:05] #link https://www.wikidata.org/wiki/Wikidata:Stable_Interface_Policy [22:48:38] gwicke: does https://www.mediawiki.org/w/index.php?title=Requests_for_comment%2FDeprecation_policy&type=revision&diff=2313462&oldid=2313460 help? [22:49:31] ok, 10 minute warning [22:49:41] hmm, not sure [22:50:06] gwicke: how about you edit it to make it more explicit? :) [22:50:09] the de-facto public part makes it sound more like it would be a deprecation [22:50:20] legoktm, do you think you have addressed all suggestions from this conversation with your edits already? or do you want to go over the proposal again later? [22:50:31] legoktm: okay, let me try [22:50:38] should it go to last call? [22:50:51] i'm for it [22:51:11] Personally I think it's ready for a last call if there aren't any objections [22:51:18] oh, i didn't check - does this supercede any pre-existing policy? [22:51:31] There is no pre-existing policy :/ [22:51:49] good. well, bad. bad it makes it easier to enact this one :) [22:51:51] Just a "guideline" that no one follows and is rather unrealistic [22:51:55] *but [22:52:26] legoktm: yes... do you know where that was written down? I can only recall some discussions on wikitech-l [22:52:41] https://www.mediawiki.org/wiki/Deprecation [22:52:52] #link https://www.mediawiki.org/wiki/Deprecation [22:52:54] so we're asking for objections to placing this RFC into a "final call" status, which means that it will be announced on wikitech-l, and then if there are no objections after a week, it will be approved by the architecture committee [22:52:59] i assuem the new policy would move there? [22:53:14] legoktm: https://www.mediawiki.org/w/index.php?title=Requests_for_comment%2FDeprecation_policy&type=revision&diff=2313467&oldid=2313462 [22:53:46] gwicke: thanks :) [22:54:06] DanielK_WMDE_: yes [22:54:36] if there are objections, the architecture committee will decide if they were appropriately addressed or if the discussion needs to be reopened [22:55:51] final call, aye or nay? [22:56:15] silence is assent [22:56:32] go for it [22:57:46] (I still think https://www.mediawiki.org/wiki/Backward_compatibility#Trunk_extensions.27_compatibility_with_old_MediaWiki_versions should be clarified but that's somewhat outside scope) [22:58:07] #info (I still think https://www.mediawiki.org/wiki/Backward_compatibility#Trunk_extensions.27_compatibility_with_old_MediaWiki_versions should be clarified but that's somewhat outside scope) [22:58:29] #info legoktm's proposal for a new deprecation policy is going to final call. [22:58:50] #action daniel to send out an email announcing the final call to wikitech-l [22:58:56] any last words? [22:59:43] thanks everyone :) [22:59:45] * gwicke says aye [23:00:02] thanks for writing this up, legoktm! [23:00:14] allright! [23:00:15] yeah, thanks [23:00:18] #endmeeting [23:00:19] Meeting ended Wed Dec 14 23:00:18 2016 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) [23:00:20] Minutes: https://tools.wmflabs.org/meetbot/wikimedia-office/2016/wikimedia-office.2016-12-14-22.04.html [23:00:20] Minutes (text): https://tools.wmflabs.org/meetbot/wikimedia-office/2016/wikimedia-office.2016-12-14-22.04.txt [23:00:20] Minutes (wiki): https://tools.wmflabs.org/meetbot/wikimedia-office/2016/wikimedia-office.2016-12-14-22.04.wiki [23:00:20] Log: https://tools.wmflabs.org/meetbot/wikimedia-office/2016/wikimedia-office.2016-12-14-22.04.log.html [23:00:22] thanks everyone!