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

Cop idea: Gemspec/TestFiles #10065

Closed
ybiquitous opened this issue Sep 6, 2021 · 3 comments · Fixed by #10105
Closed

Cop idea: Gemspec/TestFiles #10065

ybiquitous opened this issue Sep 6, 2021 · 3 comments · Fixed by #10105
Labels
feature request

Comments

@ybiquitous
Copy link
Contributor

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

I don't think it's necessary to use the test_files attribute in a gemspec in most cases, since it has been removed from the gemspec's official document and seems to be no longer recommended.

Also, if removing test_files, then we could receive smaller packed gems.

See also the discussions:

Describe the solution you'd like

I suggest adding a cop like Gemspec/TestFiles that warns the use of test_files in a gemspec.
(I'm not sure about this naming... 😅 )

Describe alternatives you've considered

I have no alternatives.

Additional context

Many gems including Rails have removed test_files to reduce a gem size.
(see also rails/rails#22272)
@koic
Copy link
Member

koic commented Sep 6, 2021 •
edited

Apart from this suggestion, the following RuboCop Packaging documentation (examples) may also be improved.
https://github.com/utkarsh2102/rubocop-packaging/blob/v0.5.1/lib/rubocop/cop/packaging/gemspec_git.rb#L10-L51

Cc @utkarsh2102

@utkarsh2102
Copy link
Member

Hi @koic,

Apart from this suggestion, the following RuboCop Packaging documentation (examples) may also be improved.
https://github.com/utkarsh2102/rubocop-packaging/blob/v0.5.1/lib/rubocop/cop/packaging/gemspec_git.rb#L10-L51

That seems like a good idea. IIUC, you propose removing any and every mention of spec.test_files, right?
Either way, would you be interested in making a PR? (I wouldn't mind even if you go ahead and commit that directly - you already have the rights :D).

@koic
Copy link
Member

koic commented Sep 6, 2021

you propose removing any and every mention of spec.test_files, right?

Right! And I can take the doc improvement of RuboCop Packaging!

koic added a commit to koic/rubocop-packaging that referenced this issue Sep 21, 2021
@koic
Remove spec.test_files from Packaging/GemspecGit's doc
db9f523 
Follow up to rubocop/rubocop#10065 (comment).

This PR removes `spec.test_files` from `Packaging/GemspecGit`'s doc.
@koic koic mentioned this issue Sep 21, 2021
Merged
@koic koic mentioned this issue Sep 21, 2021
Merged
8 tasks
koic added a commit to koic/rubocop that referenced this issue Sep 22, 2021
@koic
[Fix rubocop#10065] Add new Gemspec/TestFilesAssignment cop
e445b0b 
Fixes rubocop#10065.

This PR adds new `Gemspec/TestFilesAssignment` cop.

This cop checks that `test_files =` is not used in gemspec file.
if removing `test_files =`, then user could receive smaller packed gems.

```ruby
# bad
Gem::Specification.new do |spec|
  spec.name = 'your_cool_gem_name'
  spec.test_files = Dir.glob('test/**/*')
end

# bad
Gem::Specification.new do |spec|
  spec.name = 'your_cool_gem_name'
  spec.test_files += Dir.glob('test/**/*')
end

# good
Gem::Specification.new do |spec|
  spec.name = 'your_cool_gem_name'
end
```
koic added a commit to koic/packaging-style-guide that referenced this issue Sep 22, 2021
@koic
Remove spec.test_files from The Packaging Style Guide
7ed87d1 
Follow up to rubocop/rubocop#10065 (comment)
and utkarsh2102/rubocop-packaging#41
@koic koic mentioned this issue Sep 22, 2021
Merged
bbatsov pushed a commit that referenced this issue May 14, 2022
@koic @bbatsov
[Fix #10065] Add new Gemspec/TestFilesAssignment cop
da7506d 
Fixes #10065.

This PR adds new `Gemspec/TestFilesAssignment` cop.

This cop checks that `test_files =` is not used in gemspec file.
if removing `test_files =`, then user could receive smaller packed gems.

```ruby
# bad
Gem::Specification.new do |spec|
  spec.name = 'your_cool_gem_name'
  spec.test_files = Dir.glob('test/**/*')
end

# bad
Gem::Specification.new do |spec|
  spec.name = 'your_cool_gem_name'
  spec.test_files += Dir.glob('test/**/*')
end

# good
Gem::Specification.new do |spec|
  spec.name = 'your_cool_gem_name'
end
```
@postmodern postmodern mentioned this issue May 27, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fix #10065] Add new Gemspec/TestFilesAssignment cop koic/rubocop
3 participants
@koic @ybiquitous @utkarsh2102