<blockquote>
<blockquote>
<p><a href="https://github.com/zerebubuth" class="user-mention">@zerebubuth</a> identified tests as one thing that's missing, but there's probably other stuff.</p>
</blockquote>
<p><a href="https://github.com/zerebubuth" class="user-mention">@zerebubuth</a> : did you use <a href="https://github.com/Zverik" class="user-mention">@Zverik</a> remarks, or is this work still to do ?</p>
</blockquote>
<p>I started off with getting <code>rubocop</code> to pass, which would have addressed most of <a href="https://github.com/Zverik" class="user-mention">@Zverik</a>'s formatting remarks. I wanted to cover all the existing functionality, but didn't get anywhere near that. IIRC, I only fixed the few tests that there already were.</p>
<p>Writing tests is a good way to start; partly because there weren't enough tests before, and partly because writing tests can be a good way to understand how the code works.</p>
<p>There is a lot of new code, spread over a lot of changes to existing files and I found it hard to navigate all of it. I think there is a good argument for re-examining the goals of this code and perhaps starting from scratch, or at least laying down some broad design ideas and cleaning up the code so that it's more consistent, readable and isolated from existing code.</p>
<p>When adding this sort of functionality, I think the "Rails-y" way of doing it would be to add an <a href="http://guides.rubyonrails.org/plugins.html#add-an-acts-as-method-to-active-record"><code>acts_as</code> plugin</a>, called something like <code>acts_as_flaggable</code> or <code>acts_as_reportable</code>, and use that to keep the code for reporting problems and moderating them separate from the existing code. This would be more readable and allow the testing of that functionality separated from the context it's used in.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1268#issuecomment-283010702">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABWnLafKpvnMreiy82RIpS3cVkHD4pX0ks5rg_-bgaJpZM4Jj6LN">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABWnLVq2QfqmoofESBw8YK-cmq772Opkks5rg_-bgaJpZM4Jj6LN.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
  <link itemprop="url" href="https://github.com/openstreetmap/openstreetmap-website/pull/1268#issuecomment-283010702"></link>
  <meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/openstreetmap/openstreetmap-website","title":"openstreetmap/openstreetmap-website","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/openstreetmap/openstreetmap-website"}},"updates":{"snippets":[{"icon":"PERSON","message":"@zerebubuth in #1268: \u003e \u003e @zerebubuth identified tests as one thing that's missing, but there's probably other stuff.\r\n\u003e\r\n\u003e @zerebubuth : did you use @Zverik remarks, or is this work still to do ?\r\n\r\nI started off with getting `rubocop` to pass, which would have addressed most of @Zverik's formatting remarks. I wanted to cover all the existing functionality, but didn't get anywhere near that. IIRC, I only fixed the few tests that there already were.\r\n\r\nWriting tests is a good way to start; partly because there weren't enough tests before, and partly because writing tests can be a good way to understand how the code works.\r\n\r\nThere is a lot of new code, spread over a lot of changes to existing files and I found it hard to navigate all of it. I think there is a good argument for re-examining the goals of this code and perhaps starting from scratch, or at least laying down some broad design ideas and cleaning up the code so that it's more consistent, readable and isolated from existing code.\r\n\r\nWhen adding this sort of functionality, I think the \"Rails-y\" way of doing it would be to add an [`acts_as` plugin](http://guides.rubyonrails.org/plugins.html#add-an-acts-as-method-to-active-record), called something like `acts_as_flaggable` or `acts_as_reportable`, and use that to keep the code for reporting problems and moderating them separate from the existing code. This would be more readable and allow the testing of that functionality separated from the context it's used in."}],"action":{"name":"View Pull Request","url":"https://github.com/openstreetmap/openstreetmap-website/pull/1268#issuecomment-283010702"}}}</script>