[18:01:40] anomie: I fixed the cosmetic errors on the RevisionStore patch. Can you have a look today? [18:02:18] Also, please full in your availability in https://docs.google.com/spreadsheets/d/1_uTnQesXFREMRfhZv_z1-J6xzirM2i6rXWwAP-_nqTQ/edit#gid=0 [18:02:53] i think the plan is ambitious but not impossible. what do you think? [18:04:11] I'm guessing you're not done with the tests yet. [18:05:05] no, i'm not. but i'm interested in knowing whether you think that is the only thing that is missing for the patch to be merged. [18:05:20] i'll go through the remaining FIXMEs now, and then look into tests [18:07:22] General note: Public functions should probably have descriptions in addition to the @param. And ideally parameter descriptions on the @param lines too. [18:14:28] anomie: Sure, I'll go through all files and add what is missing. I tend to avoid rendundant/obvious documentation, though. Consider https://phabricator.wikimedia.org/P6290 [18:14:58] DanielK_WMDE: In that example, one might ask "size of what?" [18:15:26] That would be implied by the class. If it's not, the method should not be called setSize, but setWhateverSize. [18:15:41] als... what's best practice wrt @inheritDoc and @copyDoc? [18:16:11] Manually syncing documentation between interface and implementations is very annoying and error prone [18:18:54] @inheritDoc is fine [18:19:10] and we're starting to use it more often now that we have sniffs that check for missing doc comments [18:19:15] I don't think I've ever seen @copyDoc. Personally, I usually just leave it out when the whole doc block would be /** @inheritDoc */, but I'll do exactly that if someone insists. [18:20:15] I thought we only use @inheritDoc if you're over-riding (e.g. extra params on top of parent method's)? [18:26:56] Lego's sniffs say otherwise, for repos where that sniff isn't disabled. [18:27:17] James_F: hmm, that's not how we've been treating it [18:27:57] legoktm: That's a bit lame. Is phpDoc that stupid that it can't work out inheritance? [18:28:08] legoktm: It's definitely how we do things with jsduck. [18:29:42] With CodeSniffer? Not really. Maybe with phan but I haven't looked into that [18:30:03] :-( [18:30:36] To clarfy: phpDoc is fine with it. But CodeSniffer can't tell the difference between "missing doc" and "implicitly inherited doc", and warnings for the former are generally a good idea. [18:42:01] iirc, doxygen doesn't support inheritdoc, but it does support copydoc [18:42:10] we are still using doxygen, arn't we? [18:42:19] https://stackoverflow.com/questions/5543503/is-there-an-inheritdoc-equivalent-in-doxygen [18:42:30] "@copydoc is useful for the case where you want to inherit base class comments, but then add some subclass-specific details." [18:42:33] that is what i usually want [18:42:46] copy basics from the interface, provide extra info about the implementation [18:42:47] Yes, we use doxygen [18:43:19] apparently, doxgen will compy docs if you leave the derived methods completely undocumented. [18:43:24] i don't really like that... [20:15:14] we have some regexp-rewrite preprocessing phase for doxygen, that could change inheritDoc to to copyDoc if it doesn't already [20:15:36] PSR-5 says inheritDoc should be implied when there is no docblock, though [23:38:42] anomie: has the topic of API parameter length limits ever come up? [23:39:00] I was a bit surprised to find no support for that