[openstreetmap-website] Cleanup to prepare for test expansion (#135)

John Firebaugh notifications at github.com
Wed Oct 17 01:43:41 GMT 2012


Great to see other people getting involved!

I'm -0 on the magic comments. Since we externalize all our user-visible strings, it seems unlikely that we're going to need non-ASCII string constants much if at all. I'd rather add the comment as needed. I admit the error message you get if you're missing a necessary comment has room for improvement, but it's to the point and directs you to the source: `test.rb:1: invalid multibyte char (US-ASCII)`.

I'm also -0 on purely stylistic reformatting. Activity on this project is sporadic, and some branches live a long time before being restarted. To minimize merge conflicts I'd rather do style changes piecemeal as areas of code are modified for other reasons (even then it's wise to separate style changes and functional changes into separate commits).

Definitely +1 on improved tests.

---
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/135#issuecomment-9513467
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20121016/8821428b/attachment-0001.html>


More information about the rails-dev mailing list