search

YaleSTC / reservations

Manage equipment loans & reservations. Who can borrow what, for how long?
yalestc.github.io/reservations
MIT License
139 stars 58 forks source link

[1554] Refactor Reservation Model Spec #1611

Closed esoterik closed 8 years ago

esoterik commented 8 years ago

Resolves #1554

This is more of a total rewrite than a refactor; let me know if I should re-add some of the status-organized tests. I thought that this would be a better organization for the tests, and then the status context blocks became redundant after writing the unit tests, so I got rid of them.

esoterik commented 8 years ago

Hmm, the travis build seems to be failing because rubocop isn't recognizing keyword args

orenyk commented 8 years ago

Weird, it should be pulling the correct ruby version from .ruby-version according to this, so I'm not sure why we're getting the error message that we are: (Using Ruby 2.0 parser; configure using TargetRubyVersion parameter, under AllCops). Do we need to update Rubocop? Either way we can add that parameter to .rubocop.yml. I'll actually go through the code now :stuck_out_tongue:

orenyk commented 8 years ago

Ok, looked through all the additions, need to go through the deletions but that will have to be left for another time. A bunch of inline comments, overall this looks really solid but it's definitely blocked on some of our other PR's for all the mocker stuff. Let me know what you think, nice work!

orenyk commented 8 years ago

I think this is pretty close; let's figure out the testing of the renew setting and rebase. I think that this has identified a bunch of code smells and room for further refactoring but I feel like it's better to get these improvements merged in and then we can keep improving in future issues / PR's. Nice work!

orenyk commented 8 years ago

Just left some more inline notes, plus looks like we've got some merge conflicts to resolve. Back to you!

orenyk commented 8 years ago

Squash / rebase / :shipit: