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

Throw an error when time is compared to an invalid literal #6138

Merged
merged 1 commit into from
Mar 31, 2016

Conversation

jsternberg
Copy link
Contributor

A bigger refactor of these functions is needed to support #3290, but
this will work for the more common case that someone uses double quotes
instead of single quotes when surrounding a time literal.

Fixes #3932.

@jsternberg jsternberg force-pushed the js-3932-error-on-invalid-timestamp branch from b6d507b to b862a6a Compare March 28, 2016 17:29
@jsternberg jsternberg added this to the 0.12.0 milestone Mar 29, 2016
}
}
return time.Time{}
return
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Named returns seem confusing here. Why not just use returns at each case instead of assigning t and falling through?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personal preference. It was less typing and I thought it was more clear than repeating the same text everywhere. I can change it back.

@jsternberg jsternberg force-pushed the js-3932-error-on-invalid-timestamp branch 3 times, most recently from 6288298 to dccc85b Compare March 31, 2016 15:30
@jwilder
Copy link
Contributor

jwilder commented Mar 31, 2016

Needs a rebase, but LGTM. @benbjohnson

@benbjohnson
Copy link
Contributor

👍

A bigger refactor of these functions is needed to support #3290, but
this will work for the more common case that someone uses double quotes
instead of single quotes when surrounding a time literal.

Fixes #3932.
@benbjohnson benbjohnson force-pushed the js-3932-error-on-invalid-timestamp branch from dccc85b to c193bde Compare March 31, 2016 17:29
@benbjohnson
Copy link
Contributor

Rebased. Merging on 🍏

@benbjohnson benbjohnson merged commit c8d59f5 into master Mar 31, 2016
@benbjohnson benbjohnson deleted the js-3932-error-on-invalid-timestamp branch March 31, 2016 18:36
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.

3 participants