Skip to content

Add new Bundler/GemComment cop#6109

Merged
bbatsov merged 2 commits intorubocop:masterfrom
sunny:new-bundler-gem-comment-cop
Aug 4, 2018
Merged

Add new Bundler/GemComment cop#6109
bbatsov merged 2 commits intorubocop:masterfrom
sunny:new-bundler-gem-comment-cop

Conversation

@sunny
Copy link
Copy Markdown
Contributor

@sunny sunny commented Jul 19, 2018

This cop helps our team make sure all gem declarations inside a Gemfile have a comment that describes why this gem was added to the project.

Bad:

gem 'rubocop'

Good:

# Ruby styleguide enforcer.
gem 'rubocop'

The cop is disabled by default so as not to break in codebases that do not enforce this behavior.


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run rake default or rake parallel. It executes all tests and RuboCop for itself, and generates the documentation.

@sunny sunny force-pushed the new-bundler-gem-comment-cop branch from bf69179 to 3e05cc6 Compare July 19, 2018 16:11
Comment thread lib/rubocop/cop/bundler/gem_comment.rb Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove this comment.

@bbatsov
Copy link
Copy Markdown
Collaborator

bbatsov commented Jul 21, 2018

Such a cop should definitely have some whitelist of gems that don't require any explanations (generally because they are very widely used and everyone knows them or simply because they are part of some framework).

@sunny sunny force-pushed the new-bundler-gem-comment-cop branch 3 times, most recently from 4008d75 to 07223d3 Compare July 23, 2018 20:16
@sunny sunny force-pushed the new-bundler-gem-comment-cop branch from 07223d3 to 7e86351 Compare July 23, 2018 20:19
@sunny
Copy link
Copy Markdown
Contributor Author

sunny commented Jul 23, 2018

Thank you for looking at this PR! I added a Whitelist config for gems, removed the TODO and rebased against master.

@bbatsov bbatsov merged commit 344adda into rubocop:master Aug 4, 2018
@sunny
Copy link
Copy Markdown
Contributor Author

sunny commented Aug 4, 2018

Yay! Thank you for reviewing and merging this! \o/

@sunny sunny deleted the new-bundler-gem-comment-cop branch August 4, 2018 18:16
@jfelchner
Copy link
Copy Markdown
Contributor

jfelchner commented Aug 19, 2018

@sunny I LOVE the idea for this cop. ❤ How difficult would it be to add an option to this cop for an inline comment versus a comment above the line?

Example:

# This is my gem description
gem 'foo'

versus

gem 'foo' # This is my gem description

When you have a Gemfile that has a long list of gems, adding the comment above can end up making the Gemfile both extremely long, as well as more difficult to parse (inline comments can be lined up so as to be easy to scan).

@rhymes
Copy link
Copy Markdown

rhymes commented Sep 10, 2018

Does this take into account inline comments?

@sunny
Copy link
Copy Markdown
Contributor Author

sunny commented Sep 10, 2018

This does not take into account inline comments, since I don't think they help readability in Gemfiles. My "style" of Gemfiles looks more like this, usually:

# This is my gem description that is welcome to be very long.
# Also this can span multiple lines so that you can say which version is
# supported and why you decided on a specific version and what issues
# are blocking the update.
gem 'foo', git: 'git@github.com:sunny/foo', require: false

@jfelchner However, feel free to add an option to support inline comments, if you wish, it shouldn't be that hard!

@rhymes
Copy link
Copy Markdown

rhymes commented Sep 10, 2018

We use inline comments by copying the first line of the description from rubygems.org.

Example:

gem 'oj', '3.6.8'               # The fastest JSON parser and object serializer.
gem 'patron', '0.13.1'          # Ruby HTTP client library based on libcurl
gem 'pg', '1.1.3'               # Pg is the Ruby interface to the PostgreSQL RDBMS

@mikegee
Copy link
Copy Markdown
Contributor

mikegee commented Sep 10, 2018

@rhymes that's pretty cool 😎

I prefer to leave comments addressing why the gem was installed in this project and why we're on a particular branch or version, if applicable. Basically, I try to avoid comments that are duplicative with the gem's readme.

For instance:

# Get results from the FooBar API over HTTP
# 0.14 has a bug (https://github.com/patron/patron/issue/42)
gem 'patron', '0.13.1'

@rhymes
Copy link
Copy Markdown

rhymes commented Sep 10, 2018

@mikegee that makes perfect sense!

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.

6 participants