diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index 16beaec..f2111a3 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -6,6 +6,13 @@ * To change this template use File | Settings | File Templates. */ +/** + * Like I said in notes.txt, this is ALL you! I know you have frontend experience, and it's great to let it shine through here. + * + * One thing that strikes me is that your JS files and most of their functions are a bit long and complex. You might have difficulty changing things. You have lots of comments to remind you what the code does. + * Have you looked at Jasmine (https://github.com/pivotal/jasmine)? Or JS lint (http://www.jslint.com/)? Both are tools that could help you keep your JS complexity down. + */ + load("home#index", function (controller, action) { var config = { clientId: 'BCH20MFU1KWJNGTFVQHYSOQGDA42BZ5KYWGIQJW40HT4PAOT', diff --git a/app/controllers/kicks_controller.rb b/app/controllers/kicks_controller.rb index f6e4fc4..b70e1c1 100644 --- a/app/controllers/kicks_controller.rb +++ b/app/controllers/kicks_controller.rb @@ -1,4 +1,7 @@ class KicksController < ApplicationController + # Currently, this method returns this error when I navigate here: + # "There was an error saving your kick. Sorry!" If you don't want your + # users to access it, you should take it out. def index end @@ -16,6 +19,9 @@ def new end def create + # You can leverage your ActiveRecord associations to do some of this heavy + # lifting. @kick.user = @user should be all you need. Then if you want the + # @kick's username, it's just @kick.user.name @user = current_user kick_params = params.require(:kick).permit(:title, :description, :time, :location, :user_id, :latitude, :longitude, @@ -59,7 +65,8 @@ def update end end - + # In a RESTful world, this would live at kickin-it.com/users/#{user.id}/kicks + # In other words, this logic would live in the #index method def mykicks @user = User.find_by_id(current_user.id) @kicks = @user.kicks diff --git a/app/controllers/rsvp_controller.rb b/app/controllers/rsvp_controller.rb index 2fd630c..7568807 100644 --- a/app/controllers/rsvp_controller.rb +++ b/app/controllers/rsvp_controller.rb @@ -1,2 +1,3 @@ class RsvpController < ApplicationController + # Needed? If not, there's no reason to keep it. Trust in git! end diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 33a7148..262b2e6 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -13,6 +13,8 @@ def show @user = User.find_by_permalink(params[:id]) end + # TODOs are a code smell: http://c2.com/cgi/wiki?CodeSmell. File issues in + # your GitHub repo instead. ##TODO -- refactor from profile controller def update diff --git a/app/models/user.rb b/app/models/user.rb index cadf31f..367aeec 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -19,6 +19,7 @@ class User < ActiveRecord::Base :storage => :s3, :s3_credentials => "#{::Rails.root}/config/s3.yml" + # Nicely done here. Simple, declarative methods. def user_id current_user.id end diff --git a/app/views/home/index.html.erb b/app/views/home/index.html.erb index 6d8c375..908403e 100644 --- a/app/views/home/index.html.erb +++ b/app/views/home/index.html.erb @@ -6,6 +6,9 @@