Feedback PR only#24
Conversation
Tms dev it 1
…o clp_dev_model_association_tests
…o mb-dev-model-methods-2
Mb dev model methods 2
Eb dev design
…o clp_dev_model_association_tests
Tms dev trips methods
attempt to fix create trip feature test
Fix formatting on drop downs
Astute eyes!
There was a problem hiding this comment.
@tmikeschu @CPowell23 @ethanbennett @mollybrown
great work on this 🎉 i left a few comments so let me know if you have questions or if anything is unclear. keep up the hard work!!
also i LOVE all the commit messages. wooooo 🎉
| def create_cities | ||
| cities = SmarterCSV.process('db/csv/station.csv').map do |row| | ||
| row[:city] | ||
| end.uniq |
There was a problem hiding this comment.
grabbing just the unique ones. nice 👍
| end | ||
|
|
||
| def create_trips | ||
| def format_date(date) |
There was a problem hiding this comment.
this method is defined multiple times. how can we DRY up this code and only write this method once?
| end | ||
|
|
||
| end | ||
|
|
There was a problem hiding this comment.
there's a lot going on in each of the methods below. could we create a module to help us parse and clean our data? that way, the behavior will be encapsulated in a module and we can reuse it if necessary
| @@ -1,3 +1,158 @@ | |||
| require_relative '../models/station' | |||
There was a problem hiding this comment.
you shouldn't need to require your models. this is being done in config/environment.rb
| @@ -0,0 +1,10 @@ | |||
| module BikeShareAppHelper | |||
|
|
|||
| def format_date(date) | |||
There was a problem hiding this comment.
this method looks really familiar. i like that you've extracted this out to a method
| </tr> | ||
| <tr> | ||
| <td>Subscriber (annual or 30-day member)</td> | ||
| <td> <%= Trip.subscriber_breakdown %></td> |
There was a problem hiding this comment.
same here. how can we remove the need to access Trip directly in your views?
| <li>Single date with the lowest number of trips with a count of those trips: | ||
| <%= Trip.date_with_lowest_number_trips_total %> | ||
| </li> | ||
| <li>Weather on the day with the highest number of rides (mean temperature): <%= WeatherCondition.weather_on_day_with_highest_rides.first.mean_temperature_f %></li> |
| @@ -0,0 +1,36 @@ | |||
| **Group Member Names:** Mike Schutte, Ethan Bennett, Caroline Powell, Molly Brown | |||
There was a problem hiding this comment.
love that you included your DTR here 👍
| WeatherCondition.create(date: "1991-01-02", max_temperature_f: 52, mean_temperature_f: 68, min_temperature_f: 61, mean_humidity: 75, mean_visibility_miles: 7, mean_wind_speed_mph: 12, precipitation_inches: 2.3, zip_code: 94107) | ||
| end | ||
|
|
||
| it "they see a create form" do |
There was a problem hiding this comment.
i'd say the second test here is sufficient enough to test that a user can create a weather condition 👍
| require_relative '../../spec_helper' | ||
|
|
||
| describe "When a user visits the new condition path" do | ||
| before do |
5 participants
do not merge!