Skip to content

Conversation

stevepolitodesign
Copy link
Contributor

Motivation / Background

Prior to this commit, calls to gem_group would amend the Gemfile even if there was an existing gem group, resulting in duplication.

This commit prepends existing gem groups with the added gem.

Detail

Because gem_group invokes instance_eval(&block) via with_indentation, a call to gem is made. This means we needed to do the following:

  1. Check if the Gemfile contains an existing gem group in gem_group.
  2. Pass information about that existing gem group to the gem method.

Additional information

Because the Gemfile used in existing tests has a :test and :development, :test groups, we modify the existing tests to use groups not used in the Gemfile.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@@ -99,11 +99,15 @@ def gem(*args)
end
str << indentation
str << "gem #{parts.join(", ")}"
append_file_with_newline "Gemfile", str.join, verbose: false
if @gem_groups
inject_into_file "Gemfile", "\n#{str.join}", after: "group #{@gem_groups} do"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This made for the simplest implementation, but it would be nice if we append the gem group instead. I think for that, we'd need to use gsub_file which is more complicated.

Prior to this commit, calls to `gem_group` would amend the `Gemfile`
even if there was an existing [gem group][], resulting in duplication.

This commit prepends existing gem groups with the added gem.

Because the `Gemfile` used in existing tests has a `:test` and
`:development, :test` groups, we modify the existing tests to use groups
not used in the `Gemfile`.

[gem group]: https://bundler.io/guides/groups.html
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request Mar 29, 2024
Introduce `Suspenders::Cleanup::OrganizeGemfile` class and corresponding
task in an effort to reduce duplicate groups in the modified Gemfile.
This is because the [gem_group][] method does not modify existing
groups. I've opened [#49512][] in an effort to fix this, but until then,
this task will suffice.

This class is designed to be run after `suspenders:install:web`, and
does not account for all edge cases. For example, it assumes gems are
grouped by symbols (i.e. :test and not "test"), and does not account for
inline syntax:

```ruby
gem 'my-gem', group: [:cucumber, :test]
```

We could consider extracting this into a Gem (with a fun name, of
course. Maybe "Polish"), but for now, I think this simple procedural
code if just fine.

Additionally, this commit removes duplicate Rake task that was generated
with the plugin.

[gem_group]: https://api.rubyonrails.org/classes/Rails/Generators/Actions.html#method-i-gem_group
[#49512]: rails/rails#49512
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request Mar 29, 2024
Introduce `Suspenders::Cleanup::OrganizeGemfile` class and corresponding
task in an effort to reduce duplicate groups in the modified Gemfile.
This is because the [gem_group][] method does not modify existing
groups. I've opened [#49512][] in an effort to fix this, but until then,
this task will suffice.

This class is designed to be run after `suspenders:install:web`, and
does not account for all edge cases. For example, it assumes gems are
grouped by symbols (i.e. :test and not "test"), and does not account for
inline syntax:

```ruby
gem 'my-gem', group: [:cucumber, :test]
```

We could consider extracting this into a Gem (with a fun name, of
course. Maybe "Polish"), but for now, I think this simple procedural
code if just fine.

Additionally, this commit removes duplicate Rake task that was generated
with the plugin.

[gem_group]: https://api.rubyonrails.org/classes/Rails/Generators/Actions.html#method-i-gem_group
[#49512]: rails/rails#49512
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request Mar 30, 2024
Introduce `Suspenders::Cleanup::OrganizeGemfile` class and corresponding
task in an effort to reduce duplicate groups in the modified Gemfile.

This is because the [gem_group][] method does not modify existing
groups. I've opened [#49512][] in an effort to fix this, but until then,
this task will suffice.

This class is designed to be run after `suspenders:install:web`, and
does not account for all edge cases. For example, it assumes gems are
grouped by symbols (i.e. :test and not "test"), and does not account for
inline syntax:

```ruby
gem 'my-gem', group: [:cucumber, :test]
```

We could consider extracting this into a Gem (with a fun name, of
course. Maybe "Polish"), but for now, I think this simple procedural
code if just fine.

Additionally, this commit removes duplicate Rake task that was generated
with the plugin.

[gem_group]: https://api.rubyonrails.org/classes/Rails/Generators/Actions.html#method-i-gem_group
[#49512]: rails/rails#49512
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request Mar 30, 2024
Introduce `Suspenders::Cleanup::OrganizeGemfile` class and corresponding
task in an effort to reduce duplicate groups in the modified Gemfile.

This is because the [gem_group][] method does not modify existing
groups. I've opened [#49512][] in an effort to fix this, but until then,
this task will suffice.

This class is designed to be run after `suspenders:install:web`, and
does not account for all edge cases. For example, it assumes gems are
grouped by symbols (i.e. :test and not "test"), and does not account for
inline syntax:

```ruby
gem 'my-gem', group: [:cucumber, :test]
```

We could consider extracting this into a Gem (with a fun name, of
course. Maybe "Polish"), but for now, I think this simple procedural
code if just fine.

Additionally, this commit removes duplicate Rake task that was generated
with the plugin.

[gem_group]: https://api.rubyonrails.org/classes/Rails/Generators/Actions.html#method-i-gem_group
[#49512]: rails/rails#49512
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request Mar 30, 2024
Introduce `Suspenders::Cleanup::OrganizeGemfile` class and corresponding
task in an effort to reduce duplicate groups in the modified Gemfile.

This is because the [gem_group][] method does not modify existing
groups. I've opened [#49512][] in an effort to fix this, but until then,
this task will suffice.

This class is designed to be run after `suspenders:install:web`, and
does not account for all edge cases. For example, it assumes gems are
grouped by symbols (i.e. :test and not "test"), and does not account for
inline syntax:

```ruby
gem 'my-gem', group: [:cucumber, :test]
```

We could consider extracting this into a Gem (with a fun name, of
course. Maybe "Polish"), but for now, I think this simple procedural
code if just fine.

Additionally, this commit removes duplicate Rake task that was generated
with the plugin.

[gem_group]: https://api.rubyonrails.org/classes/Rails/Generators/Actions.html#method-i-gem_group
[#49512]: rails/rails#49512
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request Apr 2, 2024
Introduce `Suspenders::Cleanup::OrganizeGemfile` class and corresponding
task in an effort to reduce duplicate groups in the modified Gemfile.

This is because the [gem_group][] method does not modify existing
groups. I've opened [#49512][] in an effort to fix this, but until then,
this task will suffice.

This class is designed to be run after `suspenders:install:web`, and
does not account for all edge cases. For example, it assumes gems are
grouped by symbols (i.e. :test and not "test"), and does not account for
inline syntax:

```ruby
gem 'my-gem', group: [:cucumber, :test]
```

We could consider extracting this into a Gem (with a fun name, of
course. Maybe "Polish"), but for now, I think this simple procedural
code if just fine.

Additionally, this commit removes duplicate Rake task that was generated
with the plugin.

[gem_group]: https://api.rubyonrails.org/classes/Rails/Generators/Actions.html#method-i-gem_group
[#49512]: rails/rails#49512
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request Apr 2, 2024
Introduce `Suspenders::Cleanup::OrganizeGemfile` class and corresponding
task in an effort to reduce duplicate groups in the modified Gemfile.

This is because the [gem_group][] method does not modify existing
groups. I've opened [#49512][] in an effort to fix this, but until then,
this task will suffice.

This class is designed to be run after `suspenders:install:web`, and
does not account for all edge cases. For example, it assumes gems are
grouped by symbols (i.e. :test and not "test"), and does not account for
inline syntax:

```ruby
gem 'my-gem', group: [:cucumber, :test]
```

We could consider extracting this into a Gem (with a fun name, of
course. Maybe "Polish"), but for now, I think this simple procedural
code if just fine.

Additionally, this commit removes duplicate Rake task that was generated
with the plugin.

[gem_group]: https://api.rubyonrails.org/classes/Rails/Generators/Actions.html#method-i-gem_group
[#49512]: rails/rails#49512
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request May 10, 2024
Introduce `Suspenders::Cleanup::OrganizeGemfile` class and corresponding
task in an effort to reduce duplicate groups in the modified Gemfile.

This is because the [gem_group][] method does not modify existing
groups. I've opened [#49512][] in an effort to fix this, but until then,
this task will suffice.

This class is designed to be run after `suspenders:install:web`, and
does not account for all edge cases. For example, it assumes gems are
grouped by symbols (i.e. :test and not "test"), and does not account for
inline syntax:

```ruby
gem 'my-gem', group: [:cucumber, :test]
```

We could consider extracting this into a Gem (with a fun name, of
course. Maybe "Polish"), but for now, I think this simple procedural
code if just fine.

Additionally, this commit removes duplicate Rake task that was generated
with the plugin.

[gem_group]: https://api.rubyonrails.org/classes/Rails/Generators/Actions.html#method-i-gem_group
[#49512]: rails/rails#49512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant