User "Awjrichards" changed the status of Wikimedia.r675.

Old Status: new
New Status: ok

User "Awjrichards" also posted a comment on Wikimedia.r675.

Full URL: http://www.mediawiki.org/wiki/Special:Code/Wikimedia/675#c24955
Commit summary:

Adding TY email unsubscribe functionality plus minor stuff

Comment:

Looks good. A few things:

This will confuse every PHP developer who hasn't dealt with compiled languages 
or is otherwise ignorant of bitwise operators (lots):
<pre>
$success = $contact[ 'is_error' ] == 0 and $contact[ 'values' ][ 'is_opt_out' ] 
== 1;
</pre>
Also 'and' is rarely used and easily gets lost with all the other text. 
Generally speaking '&' is preferred. This line should be rewritten for clarity. 
Ain't no shame in a multi-line 'if' if it enhances readability. I took care of 
this for ya in r678.

It would be much preferable if the output for the unsubscribe page used a .tpl 
file and was themed in standard Drupal fashion. But, this is definitely not a 
blocker. 


_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview

Reply via email to