Mark A. Hershberger wrote: Is the MobileFrontend extension development being monitored or reviewed by any senior developers?

I took a look at the code today and it seemed incredibly strange.

Some of its layout and architecture doesn't seem to be consistent with MediaWiki conventions/style and a lot of code (particularly variable assignments) looked duplicative.

Some revisions are also apparently being pushed live without any outside review.

This isn't particularly related to the bug triage, per se, I realize.

MZMcBride _______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

On Tue, Aug 16, 2011 at 11:45 AM, Patrick Reilly <preilly(a)wikimedia.org> wrote: I was under the view that all code that went live on the cluster had to be reviewed in CR by someone else for security reasons, has this since changed?

_______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Patrick Reilly wrote: By whom? The use of a views directory containing .html.php files stuck out to me. As far as I know, this is the only extension in Wikimedia's SVN repo using this type of code structure. It looks like a holdover from the old Ruby code. Is that right? Is it going to be rewritten? Variables such as $randomButton are defined four times in the same file (extensions/MobileFrontend/MobileFrontend.php). I don't know too much about PHP, but it seemed very strange. Is there a reason for the duplication? Could it be reduced? In general, the URL parameters are odd. I'm not sure if the English Wikipedia is running the newest code, but if you visit a URL such as <http://en.wikipedia.org/w/index.php?title=Main_Page&useFormat=mobile>, none of the links within it maintain the &useFormat parameter, including and especially the links at the bottom. If you click one of the links at the bottom such as "enable images on the mobile site," it removes all parameters including the ?title= parameter. Rather than something sane and predictable for the URL parameter to view the standard (non-mobile) site (such as &useFormat=standard), the code uses ?mAction=view_normal_site. As I commented in CodeReview today, a boolean parameter (?disableImages=1) has now been complemented with a completely separate parameter (?enableImages=1) rather than using ?disableImages=0, which I find to be very strange. The HTML at the bottom of the page at <http://en.wikipedia.org/w/index.php?title=Main_Page&useFormat=mobile> appears to also be completely invalid (pasted below): ---- <div id='copyright'>Text is available under the <a rel="license" href="http://en.wikipedia.org/wiki/Wikipedia:Text_of_Creative_Commons_… ution-ShareAlike_3.0_Unported_License">Creative Commons Attribution-ShareAlike License</a><a rel="license" href="http://creativecommons.org/licenses/by-sa/3.0/" style="display:none;"></a>; additional terms may apply. See <a href="http://wikimediafoundation.org/wiki/Terms_of_use">Terms of use</a> for details.<br/> Wikipedia&reg; is a registered trademark of the <a href="http://www.wikimediafoundation.org/">Wikimedia Foundation, Inc.</a>, a non-profit organization.<br /></li><li class="noprint"><a class='internal' href="http://en.wikipedia.org/wiki/Wikipedia:Contact_us">Conta… us</a></div> </div> ---- As far as I can see, the code is defining list items without specifying a <ul> or <ol> pair. One of the <li> elements is unclosed. And there's also some very strange display:none; code in this snippet. All of this seems very odd and quirky. I believe some of this quirkiness would be mitigated by using pre-built functions and following established coding styles, rather than using DIY templates. Again, I'm wondering which senior developer (or even non-senior developer) is reviewing this code. Some clarification on that would be appreciated. I've probably written about ten lines of PHP in my life, so feel free to tell me off if these comments are out of left field and/or don't make any sense. These were just my observations from reading through some of the code today. It's incredibly dangerous to push out code without at least cursory outside review. It's an enormous risk you're taking, though it's apparently yours to take. It's a very bad situation, in my opinion. MZMcBride _______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Tomasz Finc wrote: [19:33:57] <Myra> brion: Have you been reviewing MobileFrontend? [19:34:05] <brion> Myra, nope

http://toolserver.org/~mwbot/logs/%23mediawiki/20110815.txt MZMcBride _______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Rather than something sane and predictable for the URL parameter to view the standard (non-mobile) site (such as &useFormat=standard), the code uses ?mAction=view_normal_site. As I commented in CodeReview today, a boolean parameter (?disableImages=1) has now been complemented with a completely separate parameter (?enableImages=1) rather than using ?disableImages=0, which I find to be very strange.