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 monthByName #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix monthByName #50

wants to merge 1 commit into from

Conversation

tiff
Copy link
Contributor

@tiff tiff commented Oct 18, 2013

setDate() expects an integer from 1 - 31 (not from 0 - 30).
See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/setDate

Yeah I know, javascript's date methods are quite inconsistent...

@tiff
Copy link
Contributor Author

tiff commented Oct 18, 2013

Okay, stop, don't merge this. Tests are failing. I'm checking.

@tiff
Copy link
Contributor Author

tiff commented Oct 21, 2013

Seems that the test assertions are not expecting what they should. This requires some more investigation.
The fix I proposed is definitely right and does what it should.

I worked on a separate fork for adding support for different languages (and support other date formats).
You can check it out here:
https://github.com/protonet/date

It supports German and it's quite easy to add more languages.
Also it supports time formats like "at 5 o'clock"

@matthewmueller
Copy link
Owner

Cool project!

I have a feeling there was a reason for the -1. If the tests are failing because of this PR, please update the tests.

You may also want to check if it's unrelated to this PR. In the past we've had tests that fail at certain times of the day. This should have been fixed, but one might have slipped in with a PR.

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