Always parse months as 1-indexed #358
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I could’ve sworn I fixed this with that PR. I’m kinda puzzled that this case exists.. anyways, will merge this in then. The month zero-indexing is only supposed to be done when the month is set using an integer - not when it’s a value. |
|
I merged this in my branch. Going to create a pull request including this and a fix for handling two digit years better. |
|
@rolfb, I consider it always to be better to create atomic pull requests with a single feature/improvement. Of course, if your two-digit fix depends on this one, then you have to combine them together. |
|
I agree. My two-digit fix depended on this one. |
This was referenced Apr 2, 2014
This was referenced Apr 12, 2014
amsul
added a commit
that referenced
this pull request
Apr 12, 2014
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This completes ce813fd, fixes #128 fully.
I suspect you may not like this merge, as you deliberately have added this zero-indexed parsing, but please take a look at my issue Fiddle. It results in a very strange behaviour that cost me some time to debug.
My fix may be wrong and we should do differently, but my expectation in the Fiddle was absolutely grounded, and I will argue for the usability of the library to change the behavior.