Conversation
config/routes.rb
Outdated
There was a problem hiding this comment.
Use the new Ruby 1.9 hash syntax.
Prefer single-quoted strings when you don't need string interpolation or special symbols.
Lucas-C
force-pushed
the
master
branch
6 times, most recently
from
5e05821 to
fe74084
Compare
|
What it looks like:
|
Lucas-C
force-pushed
the
master
branch
3 times, most recently
from
264dedc to
50ce003
Compare
There was a problem hiding this comment.
Redundant curly braces around a hash parameter.
Space inside { missing.
Space inside } missing.
Lucas-C
force-pushed
the
master
branch
2 times, most recently
from
786c186 to
4cbb773
Compare
|
Thx ! I'll try to find some time to review it during the week. |
|
Sorry for the delay. First, thanks a lot for your contribution ! I though about how we should add this atom feed, and I think the API would be a better place, even if it can be a surprising choice. Data presented in the feed is exactly the same that in the places endpoint ( We would then be able to have Thoughts ? ping @Lucas-C |
app/controllers/places_controller.rb
Outdated
There was a problem hiding this comment.
See #59 (comment). Could be moved to a respond_to in api/v1/places#index.
spec/factories/places.rb
Outdated
There was a problem hiding this comment.
Unused block argument - n. You can omit the argument if you don't care about it.
|
My turn to be late to answer :) Sorry ! Ok, except from your comment on not using a function in a view, I think I addressed all the points you raised. For the remaining issue with the function definition in the view:
|
3 participants
Adresses #58
The feed has been validated using feedvalidator.org and validator.w3.org/feed/check.cgi
Note that this is the very first time I'm coding in Ruby ^^
I'm open to every kind of feedback: code style, idioms, separation of concerns in Rails...