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.

Sign up for GitHub

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

Jump to bottom

Exclude *.gemspec files for Rails/TimeZone cop #432

Closed
AlexWayfer opened this issue Feb 5, 2021 · 6 comments · Fixed by #439
Closed

Exclude *.gemspec files for Rails/TimeZone cop #432

AlexWayfer opened this issue Feb 5, 2021 · 6 comments · Fixed by #439

Comments

@AlexWayfer
Copy link

Hello.

If I'm working on a gem, and it has rails as a dependency, also there is spec.date = Time.now.strftime('%Y-%m-%d') in the *.gemspec file.

Expected behavior

No offenses from Rails/TimeZone cop, because gemspec file evaluates at gem build and it will not require gem dependencies.

Actual behavior

There is an offense.

Steps to reproduce the problem

Any gem, add rails (I guess, or something related) to gem dependencies (or Gemfile maybe), use Time.now (or I guess another code for Rails/TimeZone cop) in the *.gemspec file.

RuboCop version

> bundle exec rubocop -V
1.9.1 (using Parser 3.0.0.0, rubocop-ast 1.4.1, running on ruby 2.7.2 x86_64-linux)
  - rubocop-performance 1.9.2
  - rubocop-rails 2.9.1
  - rubocop-rake 0.5.1
@andyw8
Copy link
Contributor

andyw8 commented Feb 5, 2021

I don't think I've ever seen this field used in a gemspec. The Rubygems source says:

DO NOT set this, it is set automatically when the gem is packaged.

https://github.com/rubygems/rubygems/blob/be08d8307eda3b61f0ec0460fe7fbcf647b526e6/lib/rubygems/specification.rb#L1679-L1681

@AlexWayfer
Copy link
Author

I don't think I've ever seen this field used in a gemspec. The Rubygems source says:

DO NOT set this, it is set automatically when the gem is packaged.

https://github.com/rubygems/rubygems/blob/be08d8307eda3b61f0ec0460fe7fbcf647b526e6/lib/rubygems/specification.rb#L1679-L1681

I know it, I remember, but I'm working on r18n gem, which is old and originally not mine, so there is such code, like from old "best practices" I guess, and I think I will remove it, but anyway, I think it worth to add this type of files into exclusions of this cop, because I saw such practice in other gems too.

@koic
Copy link
Member

koic commented Feb 8, 2021 •
edited
Loading

Yeah, I think both are worth it.
I think that it can make a gemspec cop to prevent setting a value to s.date =. So, I will open a PR to the core.
And I think that Rails/TimeZone cop can exclude *.gemspec by default.

@koic koic mentioned this issue Feb 8, 2021
Merged
8 tasks
koic added a commit to koic/rubocop that referenced this issue Feb 8, 2021
@koic
Add new Gemspec/DateAssignment cop
5e94c53 
Follow rubocop/rubocop-rails#432.

This PR adds new `Gemspec/DateAssignment` cop.

This cop checks that `date =` is not used in gemspec file.
It is set automatically when the gem is packaged.

```ruby
# bad
Gem::Specification.new do |spec|
  s.name = 'your_cool_gem'
  spec.date = Time.now.strftime('%Y-%m-%d')
end

# good
Gem::Specification.new do |spec|
  s.name = 'your_cool_gem'
end
```

RubyGems doesn't expect the value to be set.
https://github.com/rubygems/rubygems/blob/be08d8307eda3b61f0ec0460fe7fbcf647b526e6/lib/rubygems/specification.rb#L1679-L1681
@AlexWayfer
Copy link
Author

Yeah, I think both are worth it.
I think that it can make a gemspec cop to prevent setting a value to s.date =. So, I will open a PR to the core.
And I think that Rails/TimeZone cop can exclude *.gemspec by default.

Good idea! I support this.

koic added a commit to koic/rubocop-rails that referenced this issue Feb 9, 2021
@koic
[Fix rubocop#432] Exclude gemspec file by default for `Rails/TimeZone…
217db49 
…` cop

Fixes rubocop#432.

This PR excludes gemspec file by default for `Rails/TimeZone` cop.
@koic koic mentioned this issue Feb 9, 2021
Merged
9 tasks
@koic
Copy link
Member

koic commented Feb 9, 2021

Thank you. I opened PR #439. It will be applied in the next release :-)

@AlexWayfer
Copy link
Author

Thank you. I opened PR #439. It will be applied in the next release :-)

Thank you for your work! I'll wait and ready to help (it's not too hard to add Exclude paths, I just wanted to discuss it firstly).

bbatsov pushed a commit to rubocop/rubocop that referenced this issue Feb 12, 2021
@koic @bbatsov
Add new Gemspec/DateAssignment cop
f8f1cd4 
Follow rubocop/rubocop-rails#432.

This PR adds new `Gemspec/DateAssignment` cop.

This cop checks that `date =` is not used in gemspec file.
It is set automatically when the gem is packaged.

```ruby
# bad
Gem::Specification.new do |spec|
  s.name = 'your_cool_gem'
  spec.date = Time.now.strftime('%Y-%m-%d')
end

# good
Gem::Specification.new do |spec|
  s.name = 'your_cool_gem'
end
```

RubyGems doesn't expect the value to be set.
https://github.com/rubygems/rubygems/blob/be08d8307eda3b61f0ec0460fe7fbcf647b526e6/lib/rubygems/specification.rb#L1679-L1681
@koic koic closed this as completed in #439 Feb 14, 2021
koic added a commit that referenced this issue Feb 14, 2021
@koic
Merge pull request #439 from koic/exclude_gemspec_by_default_for_rail…
b3766c9 
…s_time_zone

[Fix #432] Exclude gemspec file by default for `Rails/TimeZone` cop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fix #432] Exclude gemspec file by default for Rails/TimeZone cop koic/rubocop-rails
3 participants
@koic @andyw8 @AlexWayfer