Feedback PR only#28
Conversation
Iteration 1
Robbie trip model
Daniel styling
"Updates delete button on trips."
Fixes delete button on stations index page
…share into checking_bretts_styling
Weather dash view
…checking_bretts_styling
Checking bretts styling
Daniel dashboard
… rake so all weather data gets imported
Br update
Removes require 'pry'
case-eee
left a comment
case-eee
left a comment
There was a problem hiding this comment.
@drod1000 @robbie-smith @riverswb good work with this 🎉 love all the commits! i'm glad y'all practiced good git workflow too. let me know if you have questions on any of my comments.
keep up the hard work 👍
| erb :"stations/new" | ||
| end | ||
|
|
||
| get '/stations/:id/detailed' do |id| |
There was a problem hiding this comment.
this is very similar to the Station show method below. this route does not follow RESTful conventions - the one on line 33 does. if i remember correctly, y'all decided not to use this one?
| :mean_wind_speed_mph, | ||
| :precipitation_inches, | ||
| presence: true | ||
| validates :zip_code, numericality: {equal_to: 94107} |
| :city, | ||
| :installation_date, presence: true | ||
|
|
||
| has_many :start_trips, class_name: :Trip, foreign_key: :start_station_id |
| Station.find(station_id) | ||
| end | ||
|
|
||
| def self.most_popular_ending_station |
There was a problem hiding this comment.
this method looks almost identical to the one on line 29. is there a way to make both of these more flexible so we can reuse the code?
| convert_time_date_to_month(sorted_by_month) | ||
| end | ||
|
|
||
| def self.month_by_month_breakdown_2015 |
There was a problem hiding this comment.
same with these monthly breakdown methods. the only difference is the initial query. can we perhaps have these methods take an argument (the year) and re-use this code?
| @@ -0,0 +1,27 @@ | |||
| require_relative '../../spec_helper' | |||
|
|
|||
| describe "Conditions/id/edit" do | |||
There was a problem hiding this comment.
we'll want to describe specific user stories, not just the path name for a specific action
| mean_temperature_f: 35, mean_visibility_miles: 10, | ||
| mean_wind_speed_mph: 20, mean_humidity: 20, | ||
| zip_code: 94107) | ||
| visit('/conditions/1') |
There was a problem hiding this comment.
how do we know that the condition you created on line 6 has an id of 1? how can we make this more flexible?
visit("/conditions/#{condition.id}")
| @@ -0,0 +1,62 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
we only want this structure of an HTML page on our layout.erb - which is always rendered on each page
| :subscription_type, | ||
| :zip_code, presence: true | ||
|
|
||
| belongs_to :starting_station, class_name: :Station, foreign_key: :start_station_id |
|
|
||
| def most_common_origination | ||
| return "" if end_trips.empty? | ||
| end_trips.most_popular_starting_station.name |
There was a problem hiding this comment.
i'd prefer if you would pass objects around (for example, this method would return a Station object) instead of data within a specific object.
what are some advantages to only passing objects around rather than specific pieces of data?
4 participants
do not merge