search

Intrepidd / working_hours

⏰ A modern ruby gem allowing to do time calculation with business / working hours.
https://github.com/Intrepidd/working_hours
MIT License
533 stars 30 forks source link

Return to opening time #25

Closed james2m closed 1 year ago

james2m commented 8 years ago

Complementary opposite method to the recent advance_to_closing_time.

jarthod commented 8 years ago

Sorry for the delay, here is my review: You should refactor return_to_opening_time to use return_to_working_time first to avoid duplicate logic. Also, the windows_between method, which is quite complex, is not tested at all. Please add specs for this or keep it for another pull request. There's also the little typo I commented in the readme. Thanks !

james2m commented 8 years ago

No problem. The windows_between was an accidental push, sorry about that. It was a bit of a spike but is all working on another branch.

I've refactored return_to_opening_time I'll re-submit when I've extracted it from the other branch. It may be 2 weeks as I'm totally maxed out until then. I've also got a bunch of stuff to stop loops and errors when moving in and out of dst, a bit late for this year, but I think it needs addressing so I will try to assemble a complete and tested pull request for that.

dmitry commented 8 years ago

Would be great to see that merged!

jarthod commented 8 years ago

@dmitry the PR has never been amended after my review so it is not ready to merge.

dmitry commented 8 years ago

@jarthod I see, but might be @james2m will have some time to complete it.

jarthod commented 8 years ago

Maybe ;) if you're also interested in this, feel free to take @james2m changes and create a pull request yourself with comments addressed.