Re: [openstreetmap/openstreetmap-website] [WIP] XML generation: nodes, ways, relations, map controller → view (#2223)

[Andy Allan]

gravitystorm commented on this pull request.



> @@ -92,13 +91,16 @@ def index
 
       # this "uniq" may be slightly inefficient; it may be better to first collect and output
       # all node-related relations, then find the *not yet covered* way-related ones etc.
+      @relations = []

`@relations = relations.uniq`
 
Or perhaps rename the `relations` variable right from the start to `@relations`, and then `@relations.uniq!` at this point

> @@ -31,7 +31,12 @@ def show
       response.last_modified = node.timestamp
 
       if node.visible
-        render :xml => node.to_xml.to_s
+        @node = node

Perhaps `@node` throughout

> @@ -31,7 +31,12 @@ def show
       response.last_modified = node.timestamp
 
       if node.visible
-        render :xml => node.to_xml.to_s
+        @node = node

There's a bunch of other places like this too - generally if you find yourself needing to rename `variable` to `@variable` it probably should have been `@variable` from the start. But it's not critical, we can always refactor this later.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/2223#pullrequestreview-255431516
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20190627/71e6aedd/attachment.html>