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

Fix arity of Date.parse and DateTime.parse #138

Merged
merged 2 commits into from Feb 19, 2015

Conversation

cgunther
Copy link
Contributor

485c4a4 introduced a regression where the method signature of Date.parse and DateTime.parse no longer matched the stdlib version. Both support 3 arguments, however the patched method only accepted one. This updates them to accept any number of arguments, passing them as is to the original method when the patch doesn't apply (not a weekday), or grabbing just the first argument when the patch does apply.

This also included a commit to rename a test file to have the _test suffix so it'd be picked up by the test runner.

If a call to Date.parse or DateTime.parse passed a second or third
argument while Timecop was loaded, the patched parse method would not
match the arity since it was only expecting a single arg.
@ThomasKoppensteiner
Copy link

+1

@cedrics
Copy link

cedrics commented Feb 19, 2015

👍

@baburdick
Copy link

+1

travisjeffery pushed a commit that referenced this pull request Feb 19, 2015
Fix arity of Date.parse and DateTime.parse
@travisjeffery travisjeffery merged commit cf0e7d9 into travisjeffery:master Feb 19, 2015
@travisjeffery
Copy link
Owner

Awesome! Thanks!

@cgunther cgunther deleted the parse-arity branch February 19, 2015 21:28
@cgunther
Copy link
Contributor Author

Sweet, thanks for the quick merge/release!

@ThomasKoppensteiner
Copy link

@cgunther thank you for fixing my fix. I forgot to check my implementation against the stdlib. I have learned my lesson. ;)

@cgunther
Copy link
Contributor Author

You're welcome!

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.

None yet

5 participants