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

Add options to customize generated gemfiles #191

Merged
merged 20 commits into from
Jul 11, 2022

Conversation

joe-sharp
Copy link
Contributor

@joe-sharp joe-sharp commented Nov 24, 2021

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
Copy link
Contributor Author

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

@nickcharlton
Copy link
Member

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
Copy link
Contributor Author

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 joe-sharp force-pushed the customize_write_gemfile branch from 861b4b4 to 6ec450c Compare March 25, 2022 19:40
@joe-sharp
Copy link
Contributor Author

@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
Copy link
Contributor Author

joe-sharp commented Mar 25, 2022

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
Copy link
Contributor Author

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

@luke-hill
Copy link

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
Copy link
Member

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!

@nickcharlton nickcharlton merged commit 40956d3 into thoughtbot:main Jul 11, 2022
@joe-sharp
Copy link
Contributor Author

Thanks so much @nickcharlton !

@joe-sharp joe-sharp deleted the customize_write_gemfile branch July 11, 2022 16:42
@deepakmahakale
Copy link

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
Copy link
Member

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

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.

4 participants