-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Bundler/OrderedGems cop does not catch gems that have a comment between them #4004
Comments
I agree. I use comments in the Gemfile like that to explain why we have to use a branch of a gem, for instance. |
Yeah, that's definitely a bug. Thanks for spotting and reporting it. |
@pocke some of us think this behavior is a bug, but it looks like you intended it to behave this way. Would you kindly contribute your opinion here? |
I reconsidered this implementation. I think the proposed behavior is correct. However, current behavior is correct, too. I have investigated several Gemfiles. Then, I can find that both usages. for example ### proposed usage
# Comment is for one gem description.
# This example is created by `rails new` command.
# Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
gem 'rails', '~> 5.0.1'
# Use sqlite3 as the database for Active Record
gem 'sqlite3'
# Use Puma as the app server
gem 'puma', '~> 3.0'
# Use SCSS for stylesheets
gem 'sass-rails', '~> 5.0'
# Use Uglifier as compressor for JavaScript assets
gem 'uglifier', '>= 1.3.0'
# Use CoffeeScript for .coffee assets and views
gem 'coffee-rails', '~> 4.2'
### A usage that is current behavior
# Command is a description for a gem group.
# Authentication
gem 'devise'
gem 'omniauth'
gem 'omniauth-github'
gem 'omniauth-twitter'
gem 'omniauth-facebook'
# Web API
gem 'octokit', '~> 4.1.0'
gem 'koala', '~> 2.2'
gem 'twitter' So, I think we should implement the proposed behavior as an option( WDYT? |
If nobody is working on this, I'll try to implement it. |
@konto-andrzeja Thanks!!! I don't have time for this issue, so that is very helpful. |
[Fix #4004] Allow not treating comment lines as gem group separators in `Bundler/OrderedGems` cop
When there is a comment between 2 gems in the Gemfile, the OrderedGems cop should still enforce that gems are listed alphabetically.
The docs make it clear that if there's an empty line between them, they don't need to be sorted. But if a comment is between them, that seems different and I would expect it to sort them.
My reasoning is as followed:
An empty line indicates that they are separate groups of gems.
A comment, on the other hand, does not indicate a separate group--it may indicate that one of the gems within this group requires some explanations. If you've enabled Style/InlineComment, then you have no other choice but to place the comment on the line above them gem, even if it's in middle of a group.
Expected behavior
Rubocop should detect 1 offense, just as it does when the comment is not there
Actual behavior
Rubocop does not detect the offense.
Steps to reproduce the problem
RuboCop version
The text was updated successfully, but these errors were encountered: