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

(#170) Add ability to remove gem from appraisal #171

Conversation

jebentier
Copy link
Contributor

@jebentier jebentier commented Oct 6, 2020

Purpose of PR

This is adding the ability to use the remove_gem declaration within an appraise block. By doing so the gem will be dropped from the generated Gemfile. This is useful in cases where an appraisal require the removal of old dependencies that are still used by the mainline source.

Example Usage

Gemfile

gem 'rails', '~> 4.2'

group :test do
  gem 'rspec', '~> 4.0'
  gem 'test_after_commit'
end

Appraisals

appraise 'rails-5' do
  gem 'rails', '~> 5.2'

  group :test do
    remove_gem 'test_after_commit'
  end
end

To produce:
gemfiles/rails_5.gemfile

gem 'rails', '~> 5.2'

group :test do
  gem 'rspec', '~> 4.0'
end

Fixes #170

Copy link
Member

@nickcharlton nickcharlton left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good.

I think it would be useful to document this addition, do you think you could add it to the README? I'm thinking that perhaps the best approach here would be to add a new section for additional options like this

Copy link

@composerinteralia composerinteralia left a comment

Choose a reason for hiding this comment

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

Nice! This is definitely something I would have used before if it existed.

lib/appraisal/bundler_dsl.rb Outdated Show resolved Hide resolved
@jebentier
Copy link
Contributor Author

@composerinteralia I've gone ahead and made the suggested edits to the new contract and updated the PR to reflect those changes.

@nickcharlton I've gone ahead and updated the README to include documentation for this new declaration

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: Nick Charlton <nick@nickcharlton.net>
README.md Outdated Show resolved Hide resolved
@jebentier
Copy link
Contributor Author

@composerinteralia would you have some time to re-review this in the coming days? I’m looking forward to getting this feature out so that we can use it at my company. If there’s anything that I can do to help with the release management of the gem as well, just let me know and I’d be happy to help.

@nickcharlton
Copy link
Member

Hi @jebentier! Sorry, that's on me. I was meaning to go ahead and merge this but it turned into a fairly busy week.

I'm going to merge this now and I'll take a look at doing a new release some time before this Friday. Thanks for your contribution!

@nickcharlton nickcharlton merged commit e095b8f into thoughtbot:master Nov 9, 2020
@jebentier jebentier deleted the add_ability_to_remove_gem_from_appraisal branch November 9, 2020 11:44
@jebentier
Copy link
Contributor Author

@nickcharlton no worries at all, it’s been rather busy here as well. Thank you for accepting this contribution.

@jebentier
Copy link
Contributor Author

@nickcharlton any update on getting this released?

@ColinDKelley
Copy link

@nickcharlton Any update on the release that will include this PR? It seems especially important since master has been documenting this new feature for 2 months now.

@ColinDKelley
Copy link

@nickcharlton Any update on the release that will include this PR?

@nickcharlton
Copy link
Member

Thanks for your patience everyone — I just did this now and so you should see version 2.4.0 available now.

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.

Ability to drop a gem in an appraisal
4 participants