thoughtbot / appraisal

A Ruby library for testing your library against different versions of dependencies.
https://thoughtbot.com
MIT License
1.25k stars 107 forks source link

Add options to customize generated gemfiles #191

Closed joe-sharp closed 1 year ago

joe-sharp commented 2 years ago

Summary

During my use of appraisal I found the need to monkey patch write_gemfile to support style conventions and to help direct other developers to Appraisal and gemfile convention guides. In this PR I add support for customizing the heading in the Appriasal-generated gemfiles and switching to single quotes. This allows users to simply set the correct keys in a special customize_gemfiles block in the Appraisals file itself. I chose to implement it this way because the Appraisals file already works like a config file in some regard so it seemed natural to be able to configure these extra options there.

Example Appraisals file

customize_gemfiles do
  {
    single_quotes: true,
    heading: <<~HEADING
      frozen_string_literal: true

      File has been generated by Appraisal, do NOT modify it directly!
      See the conventions at https://example.com/
    HEADING
  }
end

appraise 'thor' do
  gem 'thor', '~> 0.19.1'
end

Example generated gemfile

# frozen_string_literal: true

# File has been generated by Appraisal, do NOT modify it directly!
# See the conventions at https://example.com/

source 'https://rubygems.org'

gem 'thor', '~> 0.19.1'

gemspec path: '../'
joe-sharp commented 2 years ago

@nickcharlton Sorry for the ping, was hoping to get the ball rolling on this PR. Thanks in advance!

nickcharlton commented 2 years ago

Hi @joe-sharp,

Thanks for opening this and sorry for taking a very long time.

I was thinking a little bit about the syntax here; using $heading feels a little off to me (not that I can pin down why!). What do you think of an addition to the DSL? e.g.:

configure do
  heading = <<~HEADING
    frozen_string_literal: true

    File has been generated by Appraisal, do NOT modify it directly!
    See the conventions at https://example.com/
  HEADING

  single_quotes = true
end

appraise 'thor' do
  gem 'thor', '~> 0.19.1'
end

(This is probably not syntactically valid, but hopefully it demonstrates the point).

joe-sharp commented 2 years ago

Hi @joe-sharp,

Thanks for opening this and sorry for taking a very long time.

I was thinking a little bit about the syntax here; using $heading feels a little off to me (not that I can pin down why!). What do you think of an addition to the DSL? e.g.:

...

(This is probably not syntactically valid, but hopefully it demonstrates the point).

No problem at all! I will dig into this again and see if I can make something like this work, thanks!

joe-sharp commented 2 years ago

@nickcharlton thank you so much for your patience! I finally got back to this and have updated with what I hope to be a much more agreeable solution!! Additionally I updated the PR description and have added documentation. I am still open to suggestions, but I feel like we are considerably closer than when I first opened this PR.

joe-sharp commented 2 years ago

Oh, by the way, I fixed the lines that Hound complained about. However, there are quite a few single quotes used on require lines. Now some of them are not consistent, if you would like I can make a follow up PR to get the quotes consistent throughout the requires, or even the whole codebase if there are more violations.

joe-sharp commented 2 years ago

@nickcharlton friendly ping that this is still in need of review. Thanks!

luke-hill commented 2 years ago

This seems awesome and would fix a load of "minor" tech debt we have in cucumber-rails.

Not saying we're the biggest customer, but if this can go in we can fix two issues for the price of one (We're downgraded due to the set issue from last time).

nickcharlton commented 1 year ago

I appreciate all of the pings (and follow up) here, thank you everyone! I've been super busy and open source is, unfortunately, the one which gets dropped 😢 .

I'm finally merging this. I can't promise when I'll do a release but at least we're one step closer to that possibility!

joe-sharp commented 1 year ago

Thanks so much @nickcharlton !

deepakmahakale commented 1 year ago

Any plan on releasing a new gem version with this code? Also, is it possible to support using the gemfile variable in the header? Use Case:

This is the header we want

# frozen_string_literal: true
#
# BUNDLE_GEMFILE="gemfiles/rails_5_1.gemfile" bundle exec rake test

So something like

customize_gemfiles do
  {
    heading: <<~HEADING
      frozen_string_literal: true

      BUNDLE_GEMFILE="%{gemfile}" bundle exec rake test
    HEADING
  }
end
nickcharlton commented 1 year ago

@deepakmahakale Could you open a new issue for that (or even better a PR!)? Seems like a good idea!