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

New cop to check for the use of Time.zone= method. #458

Closed
olivierbuffon opened this issue Apr 7, 2021 · 2 comments · Fixed by #459
Closed

New cop to check for the use of Time.zone= method. #458

olivierbuffon opened this issue Apr 7, 2021 · 2 comments · Fixed by #459

Comments

@olivierbuffon
Copy link
Contributor

olivierbuffon commented Apr 7, 2021

Is your feature request related to a problem? Please describe.

I've been through a code base lately that use to use Time.zone= to temporarily update the application time zone for a specific operation.
This might cause unexpected behaviors if the time zone is not set back to its original value.
Most of the time I've seen this pattern used in specs like:

before do
  Time.zone = 'EST'
end

Describe the solution you'd like

In order to avoid some issues related to the application time zone update, I would recommend the usage of Time.use_zone(time_zone, &block) instead. So we could be sure the time zone always reverts back to its initial value.
Its also really simple to use in specs using the around hook:

around(:example) do |example|
  Time.use_zone('EST') { example.call }
end

What do you think?

Describe alternatives you've considered

N/A

Additional context

N/A

@andyw8
Copy link
Contributor

andyw8 commented Apr 8, 2021

Setting Time.zone within the app's config is valid, so I suppose this cop should be restricted to test code?

olivierbuffon added a commit to olivierbuffon/rubocop-rails that referenced this issue Apr 8, 2021
This cop checks for the use of Time.zone= method.
Built on top of this nice Thoughtbot article (https://thoughtbot.com/blog/its-about-time-zones)
olivierbuffon added a commit to olivierbuffon/rubocop-rails that referenced this issue Apr 8, 2021
This cop checks for the use of Time.zone= method.
Built on top of this nice Thoughtbot article (https://thoughtbot.com/blog/its-about-time-zones)
@olivierbuffon
Copy link
Contributor Author

@andyw8 Thanks for your feedback.
Well I think you're right! Since most of the time I've seen this in tests I agree this might be a better approach.
I'll update my code and open a PR 😃

olivierbuffon added a commit to olivierbuffon/rubocop-rails that referenced this issue Apr 8, 2021
This cop checks for the use of Time.zone= method.
Built on top of this nice Thoughtbot article (https://thoughtbot.com/blog/its-about-time-zones)
olivierbuffon added a commit to olivierbuffon/rubocop-rails that referenced this issue Apr 9, 2021
This cop checks for the use of `Time.zone=` method.
Built on top of this nice Thoughtbot article (https://thoughtbot.com/blog/its-about-time-zones)
olivierbuffon added a commit to olivierbuffon/rubocop-rails that referenced this issue Apr 9, 2021
This cop checks for the use of `Time.zone=` method.
Built on top of this nice Thoughtbot article (https://thoughtbot.com/blog/its-about-time-zones)
olivierbuffon added a commit to olivierbuffon/rubocop-rails that referenced this issue Apr 11, 2021
This cop checks for the use of `Time.zone=` method.
Built on top of this nice Thoughtbot article (https://thoughtbot.com/blog/its-about-time-zones)
@koic koic closed this as completed in #459 Apr 12, 2021
koic added a commit that referenced this issue Apr 12, 2021
[Fix #458] Add new `Rails/TimeZoneAssignment` cop
koic added a commit to koic/rubocop-rails that referenced this issue Jun 7, 2021
Fixes rubocop#495

This PR adds new `Rails/I18nLocaleAssignment` cop.
It has a similar purpose to `Rails/TimeZoneAssignment` cop.
rubocop#458
koic added a commit to koic/rubocop-rails that referenced this issue Jun 8, 2021
Fixes rubocop#495

This PR adds new `Rails/I18nLocaleAssignment` cop.
It has a similar purpose to `Rails/TimeZoneAssignment` cop.
rubocop#458
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 a pull request may close this issue.

2 participants