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

Fix offense message from Rails/TimeZone. #1777

Merged
merged 3 commits into from
Apr 9, 2015

Conversation

mzp
Copy link
Contributor

@mzp mzp commented Apr 9, 2015

Now, Rubocop generate "Use #Time.zone.[:new] instead." for following code.

Time.new(2014, 1, 1)

But Time.zone.new does not exist. We should use Time.zone.local.

As requested in the contributing guidelines:

$bundle exec rubocop -V
0.30.0 (using Parser 2.2.0.3, running on ruby 2.2.0 x86_64-linux)

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 9, 2015

Please, remove the . from the commit message's title and add an entry to the changelog regarding this bugfix.

@mzp mzp force-pushed the fix_time_zone_offense_message branch 2 times, most recently from c3621a1 to 5ba02d7 Compare April 9, 2015 14:15
@mzp
Copy link
Contributor Author

mzp commented Apr 9, 2015

I fix it and rebase on master.

@mzp mzp changed the title Fix offense message for Rails/TimeZone. Fix offense message from Rails/TimeZone. Apr 9, 2015
Rails/TimeZone rule generate offense message like 'Use #Time.zone.new instead'.

But `Time.zone.new` is not exist. We should use `Time.zone.local`.
@mzp mzp force-pushed the fix_time_zone_offense_message branch from 5ba02d7 to aa70f41 Compare April 9, 2015 14:21
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 9, 2015

Btw, Time.zone.local would be the equivalent only in case there are any arguments. Otherwise the equivalent would be Time.zone.now or Time.current.

@mzp
Copy link
Contributor Author

mzp commented Apr 9, 2015

Oh, I miss it. I add a commit to suggest Time.zone.now when {DateTime, Time}.new is called without any arguments.

$ bundle exec rubocop -R ~/tmp/bar.rb
Inspecting 1 file
C

Offenses:

/home/mzp/tmp/bar.rb:1:6: C: Do not use Time.new without zone. Use #Time.zone.now instead.
Time.new
     ^^^
/home/mzp/tmp/bar.rb:3:6: C: Do not use Time.new without zone. Use #Time.zone.local instead.
Time.new(2014, 1, 1)
     ^^^


add_offense(node, :selector,
format(MSG,
"#{klass}.#{method_name}",
"#Time.zone.#{method_name}")
"#Time.zone.#{safe_method_name}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

There should not be # before Time and the code snippets should surrounded in backticks, so they're properly coloured in the terminal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fix it. Now properly colored in the terminal.

screen shot 2015-04-10 at 12 33 24 am

bbatsov added a commit that referenced this pull request Apr 9, 2015
Fix offense message from Rails/TimeZone.
@bbatsov bbatsov merged commit 97f5eb4 into rubocop:master Apr 9, 2015
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 9, 2015

👍 Thanks!

@mzp mzp deleted the fix_time_zone_offense_message branch April 9, 2015 22:30
@mzp
Copy link
Contributor Author

mzp commented Apr 9, 2015

Thanks!

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.

2 participants