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

Allow passing holidays with each invocation. #180

Closed

Conversation

mstate
Copy link
Contributor

@mstate mstate commented Feb 17, 2018

Modified code, test and documentation to allow passing a holiday or array of holidays with each invocation. This is particularly useful when dealing with potentially different regional holidays during each invocations.

@bokmann
Copy link
Owner

bokmann commented May 18, 2018

Wow... I like this solution to a problem I've had (calculating backwards after snow days have happened), and it was less impactful that I would have considered.

It has a merge conflict with another pull I just merged, but if you resolve that it'll go into the next version!

Thanks for the first-time commit! Don't forget to add yourself as a contributor to the readme!

@RST-J
Copy link

RST-J commented Nov 17, 2018

We recently had a problem with a local holiday in our region that does not exist in other regions of Germany. It would be solvable with BusinessTime::Config.with but we were not aware of that method and the solution implemented here is less verbose.

Inspired by this I thought of an direct integration of the holidays gem into business_time. But I am not sure whether you would be up for it @bokmann? Reading the Not-Todo section and the following one about stability and change suggest rather not. But I thought I'd ask first before considering doing it in a fork.

@Laykou
Copy link

Laykou commented Nov 8, 2021

Do you think this could be merged, please?

@rmm5t
Copy link
Collaborator

rmm5t commented Nov 23, 2021

Do you think this could be merged, please?

@Laykou This PR has merge conflicts that would need to be resolved first.

@Laykou
Copy link

Laykou commented Dec 9, 2021

@mstate Would you be willing to resolve the conflicts, please?

@cadenforrest
Copy link
Contributor

I took a stab at resolving the conflicts in #217

@rmm5t rmm5t closed this in #217 Jun 26, 2022
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.

6 participants