"Awjrichards" changed the status of Wikimedia.r1176 to "fixme" and commented it.
URL: http://www.mediawiki.org/wiki/Special:Code/Wikimedia/1176#c30251
Old Status: new
> New Status: fixme
Commit summary for Wikimedia.r1176:
Initial commit of the globalcollect audit module.
This doesn't currently work, and will die as soon as it puts a stomp array
together. Awesome.
Awjrichards's comment:
<pre>
$cmd = "grep '<ORDERID>$order_id</ORDERID>'
/home/khorn/logs/globalcollect/20111221_iop.log";
echo $cmd . "\n";
$ret = array();
exec( $cmd, $ret, $errorlevel );
$proof = array();
</pre>
You should wrap $cmd in something like escapeshellcmd()
(http://us.php.net/manual/en/function.escapeshellcmd.php). I realize the way
it's currently being used is benign, it is good practice and will potentially
be crucial when you make the file name something dynamic.
<pre>
$stomp_field_map = array( ...
</pre>
You might punch me in the face for this, but I think you should abstract out
all or at least nearly all stomp-related stuff from both this module as well as
DonationInterface and put it into its own library - similar to what Jeremy's
doing with the StandaloneGlobalCollectAdapter. Or perhaps they could be part of
a shared 'WMF fundraising' library. The library/libraries could then be used
anywhere in the fundraising pipeline and you could be certain that you won't
have feature drift - it will afford you /a lot/ of flexibility n the future.
Duplicating this stuff in both DonationInterface and here will trip you up in
the future - I guarantee it. I realize that might complicate getting this done
quickly, so perhaps now is not the right time to do it. But there might never
be a 'good' time to do it. Just something to think about!
_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview