Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the ability to use different weekend days. #14

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

SaharZehavi
Copy link

@SaharZehavi SaharZehavi commented Sep 13, 2017

for fri-sat weekends use.

use example:
bizniz.setWeekendDays([5,6]).addWeekDays(new Date(), 5) // will consider friday as the weekend and not sunday.

@jamesplease
Copy link
Owner

Thanks @SaharZehavi ! This looks promising.

Two things:

  1. I'm not a fan of the "global" weekend setting. If we want an API like bizniz.setWeekendDays([5,6]).addWeekDays(new Date(), 5), it should use a Constructor and return an entirely separate bizniz. That way, you could set the weekend days, and then later do:

bizniz.addWeekDays()

and it would use the default weekends. Does that make sense?

Also, would you like to try your hand at adding unit tests?

@SaharZehavi
Copy link
Author

that was what made sense to me.
if i would have any more time to work on it, i'll change it/add tests to it. can't really commit to it.

@jamesplease
Copy link
Owner

Sure thing. I wish I had time to help out, but time is seeming scarce these days for me!

Future visitors: if you want to see this get merged, feel free to jump in and update the API / write some tests. You can just make a PR into this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants