[teampractices] Code review before writing new code

[Matthew Flaschen]

On 06/11/2015 09:40 AM, Quim Gil wrote:
> We had this before, but it caused two problems:
>
> * keeping
> https://wikitech.wikimedia.org/wiki/Key_Wikimedia_software_projects up
> to date and in sync with korma was not easy

This is a solvable problem.  For example, it could be added to 
https://www.mediawiki.org/wiki/Writing_an_extension_for_deployment and 
people can be reminded of it.

> * if non-Wikimedia repos receive little attention when they are mixed
> with the rest, imagine what happens when we take them out our sight.

The apparent policy is that any open source project related to MediaWiki 
can be in Gerrit.

I support that, but it doesn't mean I'm willing or able to review it.

* I'm naturally going to be more likely to review WMF-deployed software, 
both since I work for the WMF, and because it generally has more impact.

* There is a lot of non-WMF deployed software that I am never going to 
review, and this should not negatively affect our metrics.

** I am probably never going to review extensions like 
https://www.mediawiki.org/wiki/Extension:Google_Analytics_Integration 
and https://www.mediawiki.org/wiki/Extension:Google_AdSense (certainly 
not on WMF time), both for philosophical reasons and because it's not 
relevant to the WMF.

** There are a lot of other non-WMF extensions that are probably 
perfectly useful, but I just don't have time to learn.

We want these metrics to be fair if people are to take them into account.

> Most of these repos only have 1-2 patches. Cleaning the way is
> relatively easy in half of the cases, especially if we agree that the
> pragmatic "let's clean the backlog in non-critical areas" is preferable
> to the impossible goal of "let's be absolutely nice with every single
> patch in some unmaintained repo that is stuck during more than a year".

I agree, but for non-WMF extensions, that job should primarily fall to 
the maintainer(s) of that extension.  If there is no maintainer, then 
maybe it should be moved to yet a third list ("WMF-deployed", "not 
WMF-deployed", "Completely unmaintained").

> I have been marking old quiet changesets in likely abandoned repos with
> "[WIP]" to get them out of the way, sometimes with the help of Jenkins
> bringing a -1 after the change.

WIP should ideally reflect the author's intentions ("do I the author of 
the patch feel it's done?", not whether it's likely to be merged).

> After a bit of superficial cleaning, the WMF maintained repos that need
> more attention will start to emerge. As of now:
>
> 12. UploadWizard
> 16. Wikistats
> 21. MassMessage
> 22. Campaigns
> 30. Echo
>
> These names start sounding familiar, right?

Yes (except I really know almost nothing about wikistats), but none of 
them were even in the top 10 apparently when you sent the email.  It's 
great to have non-WMF extensions in Gerrit, and it's fine to track the 
metrics.  They should just be tracked separate if it's to be part of WMF 
team accountability.

Matt