"Ottomata" posted a comment on MediaWiki.r111444.
URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/111444#c31398

Commit summary for MediaWiki.r111444:

- Adds a "udplog_escaped_content_type" variable for url encoding the content 
type header

Ottomata's comment:

Looks like it'll work, and is probably a very safe change.

I have to say though, ngx_http_udplog_escaped_content_type_variable() is an 
exact copy/paste of ngx_http_udplog_escaped_user_agent_variable(), except with 
one variable name changed and the use of r->headers_in.content_type instead of 
r->headers_in.user_agent.  Ideaaaaaally, this logic would be DRYed up, but you 
know, this is C and a quick change to make this work, and DRYing it might break 
other unknown things.  

I'd add some comments into the new function explaining that you know is is 
copy/pasted and that ideally it would be DRYed, just so that some future dev 
doesn't come in and say "AGH why didn't this guys DRY this up!?"

Anyway, looks good to me.


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

Reply via email to