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

Reject invalid dates #3

Merged
merged 1 commit into from
Jul 15, 2016
Merged

Conversation

erikkemperman
Copy link
Member

@phated
Copy link
Member

phated commented Jul 15, 2016

This would normalize to a timestamp. So the method name would be weird. I think, based on #4, that it would take lenient input (dates, numbers, number objects) and return a Date object. We could mirror that with a .timestamp() method that returns the number timestamp.

Also, do you think we should use the vali-date module to do date object validation?

@erikkemperman
Copy link
Member Author

This PR doesn't change the return type, just the boolean function to decide accept/reject of the given value. In response to the vinyl-fs issue about consistent dates.

Looking at this though, got me thinking about lenient in / strict out, and then I made that issue here, so I guess my timing was confusing but this PR is separate.

I have something working for that other issue as well but would like to add a bit more.

@erikkemperman
Copy link
Member Author

PS vali-date would not be necessary I think. Wasteful even, because it checks validity by actually coercing to Date... Which it then throws away and returns true or false. For us to coerce again.

@phated
Copy link
Member

phated commented Jul 15, 2016

Oh yeah, I was thinking about that method incorrectly. This looks good. I'll merge and bump patch version.

@phated phated merged commit b3c3402 into gulpjs:master Jul 15, 2016
@erikkemperman
Copy link
Member Author

Thanks!

@erikkemperman erikkemperman deleted the reject-invalid-dates branch July 16, 2016 09:35
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

2 participants