search

sky-uk / ruby-bootcamp

Other
18 stars 22 forks source link

Exercise:10 #13

Closed lashd closed 9 years ago

lashd commented 9 years ago

This is a first crack at #10. @joshnesbitt could you give this a look?

joshnesbitt commented 9 years ago

Do we want to be careful merging the solution @lashd? Will take a proper look at this when I get a chance.

lashd commented 9 years ago

I don't think that I have merged any solution code? Definitely a mistake if I have.

joshnesbitt commented 9 years ago

@lashd sorry you haven't merged it yet, but this PR contains a solution that's going to get merged into master right?

lashd commented 9 years ago

@joshnesbitt don't think there is any solution code in there. If you are reffering to the specs? I originally added those because I thought that they could run code coverage and add the missing tests. I then replaced code coverage for rubocop and but left them in because there was warning thrown by rubocop that I wanted them to switch off.

We could take these out and do something dedicated to wiring in coverage?

joshnesbitt commented 9 years ago

@lashd ah OK I see what you're doing. Should be fine then. Sorry not able to dedicate proper time on this now but will review properly soon.

lashd commented 9 years ago

We could add a part b) to the first exercise, getting them to customise the rspec tasks and turn on code coverage then add this missing test? (the missing tests are checking that exceptions are thrown)

e.g.

RSpec::Core::RakeTask.new(:spec) do
  ENV['coverage'] = true
end

Then in the spec help they could check for this environment variable to require the necessary simplecov bits and pieces?

lashd commented 9 years ago

@joshnesbitt no problem - when ever you get the time.