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

Time.current.strftime should not be an offense #2026

Closed
filipegiusti opened this issue Jul 9, 2015 · 5 comments
Closed

Time.current.strftime should not be an offense #2026

filipegiusti opened this issue Jul 9, 2015 · 5 comments

Comments

@filipegiusti
Copy link

On code

timestamp = Time.current.strftime('%Y%m%d_%H%M%S')
File.open("report-#{timestamp}", 'w')

at rubocop version

$ rubocop -v
0.32.1

I'm getting

Do not use Time.strftime without zone. Use one of Time.strftime.in_time_zone, Time.strftime.utc, Time.strftime.iso8601, Time.strftime.jisx0301, Time.strftime.rfc3339, Time.strftime.to_i, Time.strftime.to_f instead.

But I don't want to have a Time object (as returned by in_time_zone), I only want my formatted string and I'm using current to set the right timezone.

@mikegee
Copy link
Contributor

mikegee commented Jul 15, 2015

I confirmed the reported behavior in the latest master just now.

I like to use Time.current, too. It is timezone aware and should be supported by Rubocop, but isn't yet. http://api.rubyonrails.org/classes/Time.html#method-c-current

@ClairePitman
Copy link

The docs say that if Time.zone or config.time_zone are not set it just returns Time.now.
I think that this would be ok when EnforcedStyle is acceptable but otherwise it seems like a step back toward Time.now.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 16, 2015

//cc @palkan

I've never received more issue reports than about this cop. Seems there's no good way to check for the time zone problems reliably, so I guess we should just disable it by default.

@palkan
Copy link
Contributor

palkan commented Jul 16, 2015

@bbatsov Maybe you're right. Too much edge cases.

But nevertheless I'll add support for Time.current as acceptable.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 16, 2015

Yep, you should.

bbatsov added a commit that referenced this issue Jul 16, 2015
[Fix #2026] Allow `Time.current` when style is 'acceptable'
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

No branches or pull requests

5 participants