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 Bundler/OrderedGems cop bug #5623

Conversation

colorbox
Copy link
Contributor

@colorbox colorbox commented Mar 3, 2018

Summary

This PR improves Bundler/OrderedGems autocorrect behavior on the
specific situation.
When group includes duplicate gems, and one of duplicate gems is
in the first line of group, and one of them has
offense Bundler/OrderedGems.
The cop replace the gem out of group and the gem in group.

Before

For example, the target Gemfile is like below.

gem 'a'

group :development do
  gem 'b'
  gem 'c'
  gem 'b'
end

Expected

Expected result of rubocop -a --only Bundler/OrderedGems
is like below.

gem 'a'

group :development do
  gem 'b'
  gem 'b'
  gem 'c'
end

Actual

But actual result is like below.

gem 'b'

group :development do
  gem 'a'
  gem 'b'
  gem 'c'
end

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.

@colorbox colorbox force-pushed the fix_bundler_ordered_gem_cop_bug_when_group_includes_duplicate_gems branch from ec405b6 to db45946 Compare March 4, 2018 00:03
CHANGELOG.md Outdated
@@ -30,6 +30,7 @@

### Bug fixes

* [#5623](https://github.com/bbatsov/rubocop/pull/5623): Fix `Bundler/OrderedGems` when a group includes duplicate gems. ([@colorbox][])
Copy link
Collaborator

Choose a reason for hiding this comment

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

The changelog entry should be moved, as a new release was cut a few days ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. 🙏 Okay, I'l fix it.

@colorbox colorbox force-pushed the fix_bundler_ordered_gem_cop_bug_when_group_includes_duplicate_gems branch from 06ac104 to 107d8f3 Compare March 6, 2018 13:01
  ## Summary

  This PR improves `Bundler/OrderedGems` autocorrect  behavior on the
  specific  situation.
  When group includes duplicate gems, and one of duplicate gems is
  in the first line of group, and one of them has
  offense `Bundler/OrderedGems`.
  The cop replace the gem out of group and the gem in group.

  ### Before

  For example, the target Gemfile is like below.

  ```ruby
  gem 'a'

  group :development do
    gem 'b'
    gem 'c'
    gem 'b'
  end
  ```

  ### Expected

  Expected result of `rubocop -a --only Bundler/OrderedGems`
  is like below.

  ```ruby
  gem 'a'

  group :development do
    gem 'b'
    gem 'b'
    gem 'c'
  end
  ```

  ### Actual

  But actual result is like below.

  ```ruby
  gem 'b'

  group :development do
    gem 'a'
    gem 'b'
    gem 'c'
  end
  ```
@colorbox colorbox force-pushed the fix_bundler_ordered_gem_cop_bug_when_group_includes_duplicate_gems branch from 107d8f3 to c10b291 Compare March 6, 2018 13:57
@bbatsov bbatsov merged commit 8ec6cf6 into rubocop:master Mar 8, 2018
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 8, 2018

Thanks!

@colorbox colorbox deleted the fix_bundler_ordered_gem_cop_bug_when_group_includes_duplicate_gems branch March 8, 2018 04:50
This was referenced Mar 21, 2018
This was referenced Mar 21, 2018
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.

3 participants