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

Raise ArgumentError when an invalid form is passed to String#to_time #24702

Closed
wants to merge 1 commit into from

Conversation

yui-knk
Copy link
Contributor

@yui-knk yui-knk commented Apr 23, 2016

String#to_time accepted any an argument as form and
behaved like as if :local was passed. This change notifies
developers they passed an invalid argument.

`String#to_time` accepted any an argument as `form` and
behaved like as if `:local` was passed. This change notifies
developers they passed an invalid argument.
@rails-bot
Copy link

r? @kaspth

(@rails-bot has picked a reviewer for you, use r? to override)

@pixeltrix
Copy link
Contributor

I don't think there's any real need to raise an error here - in fact I'd rather we deprecated these arguments altogether but I need to see how often they're used.

@jeremy
Copy link
Member

jeremy commented Apr 23, 2016

@pixeltrix We recently added the corresponding errors in #24552

@yui-knk
Copy link
Contributor Author

yui-knk commented Apr 24, 2016

BTW I agree "we deprecated these arguments". Can we do this on Rails 5?

@sgrif
Copy link
Contributor

sgrif commented Apr 24, 2016

Given that the feature freeze was over a month ago, it'd be better to push any API changes until after Rails 5 unless they're related to a release blocker.

@pixeltrix
Copy link
Contributor

@jeremy the problem is that the :local form is a bit weird now when to_time is preserving timezones, e.g:

>> "2016-04-24T00:00:00Z".to_time(:local)
=> "2016-04-24T00:00:00.000Z"

A better way is to use getlocal explicitly:

>> "2016-04-24T00:00:00Z".to_time.getlocal
=> "2016-04-24T01:00:00.000+01:00"

@sgrif can you clarify whether it's a code freeze or a feature freeze unless related to a release blocker?

@kaspth
Copy link
Contributor

kaspth commented Apr 24, 2016

r? @pixeltrix

@rails-bot rails-bot assigned pixeltrix and unassigned kaspth Apr 24, 2016
@rails-bot
Copy link

rails-bot bot commented Dec 18, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 18, 2019
@rails-bot rails-bot bot closed this Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants