<p>Sorry for not providing feedback more quickly. I've had a look at this, and here are my suggestions:</p>
<ul>
<li>
<p>Avoid the topic of default rendering formats in this PR. It creates a breaking API change in some edge cases (as per <a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="380901348" data-permission-text="Issue title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/2065" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/2065/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/pull/2065">#2065</a>) so best dealt with separately. This means your controllers shouldn't contain <code>respond_to</code> blocks and the tests then won't need changes.</p>
</li>
<li>
<p>Don't wrap objects in arrays unnecessarily. For example <code>@nodes = [node]</code>. The relevant view should just handle an <code>@node</code> object instead.</p>
</li>
<li>
<p>Try to avoid creating arrays for no reason. For example, after refactoring your code becomes:</p>
</li>
</ul>
<pre><code>      @elems = []

      visible_elements.each do |element|
        @elems << element
      end
</code></pre>
<p>... which isn't doing anything useful!</p>
<ul>
<li>
<p>If you're not sure how to handle the caches for users etc (and to be honest, I don't know either) then just leave these methods and come back to them in a separate PR. I'd prefer to see one PR that covers all the straightforward refactoring and is easy to review, and a second PR that covers tough things but is nice and short.</p>
</li>
<li>
<p>I think there's need for <code>.empty?</code> guards when using <code>.each</code> (in  app/views/api/map/_node.xml.builder)</p>
</li>
<li>
<p>Avoid using <code>@variables</code> in partials. The partials are easier to reuse/understand if you pass in the objects at the point they are rendered. For example, you have <code>@bounds</code> in the _bounds.builder partial, but it would be better to call it from the index.xml.builder with e.g. <code>  osm << (render(:partial => "bounds", :object => @bounds) || "")</code> and then just use <code>bounds</code> in the bounds partial.</p>
</li>
<li>
<p>Check out <a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="460937103" data-permission-text="Issue title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/2278" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/2278/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/pull/2278">#2278</a> since it applies to many of the partial renderings that you have in this PR.</p>
</li>
<li>
<p>Do you really need <code>|| ""</code> in all these render calls? I suspect (but haven't tested) that most calls will work without them.</p>
</li>
</ul>

<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/2223?email_source=notifications&email_token=AAK2OLM6ZFAAR7POTAELJITP4NWZXA5CNFSM4HLMFYO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYTR4LY#issuecomment-505880111">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLN4PIJ5CHGF5LGZ5G3P4NWZXANCNFSM4HLMFYOQ">mute the thread</a>.<img src="https://github.com/notifications/beacon/AAK2OLMV5RR3MQGPHK5ZRXDP4NWZXA5CNFSM4HLMFYO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYTR4LY.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/2223?email_source=notifications\u0026email_token=AAK2OLM6ZFAAR7POTAELJITP4NWZXA5CNFSM4HLMFYO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYTR4LY#issuecomment-505880111",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/2223?email_source=notifications\u0026email_token=AAK2OLM6ZFAAR7POTAELJITP4NWZXA5CNFSM4HLMFYO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYTR4LY#issuecomment-505880111",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>