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 support for ruby file: … Gemfile syntax #219

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

tisba
Copy link
Contributor

@tisba tisba commented Feb 12, 2024

I was looking for a way to use ruby file: '.ruby-version' in my gem for testing. Using this Appraisals file current yields an error:

appraise "foo" do
  ruby file: '../.ruby-version'
  
  gem "faraday-follow_redirects", "0.3.0"
end

After digging around a little, I noticed that Appraisal::BundlerDSL#ruby_version is emitting a block when the Gemfile DSL ruby method is used. Currently, this is emitted – notice that this is not a method argument, but a block 🐛:

ruby {:file=>"../.ruby-version"}

So instead, we have to add () to ensure the arguments stay arguments:

ruby({:file=>"../.ruby-version"})

PS: I would like to add specs, but I'm unable to run specs (see #218).

@tisba tisba marked this pull request as ready for review February 13, 2024 08:49
@tisba
Copy link
Contributor Author

tisba commented Feb 13, 2024

Hey @nickcharlton, maybe you can take a look at this one?

I copied the style of the acceptance test, so I'm not sure what do do about the hound bot comments 🤔

I opted to preserve the generated output when ruby is called with a String: I figured this was easy enough. If you feel differently about it, let me know. Same with testing this on the acceptance-level. Happy to move the spec elsewhere!

Locally the tests are all green as discussed in #218, using Bundler 2.3.27 via:

bundle install
bundle _2.3.27 _ update --bundler
bundle install
rspec

@thoughtbot thoughtbot deleted a comment from hound bot Feb 15, 2024
@thoughtbot thoughtbot deleted a comment from hound bot Feb 15, 2024
@thoughtbot thoughtbot deleted a comment from hound bot Feb 15, 2024
@thoughtbot thoughtbot deleted a comment from hound bot Feb 15, 2024
@thoughtbot thoughtbot deleted a comment from hound bot Feb 15, 2024
@thoughtbot thoughtbot deleted a comment from hound bot Feb 15, 2024
@nickcharlton
Copy link
Member

Great, thanks for doing this! I tested it out locally, and it's working for me with Bundler 2.3.27.

I just deleted the Hound comments — I've been bit by bit moving to standard and this is on my list to do that here too. As it's copying existing styles anyway, I think it's fine.

Previously, this would yield an error:

    appraise "foo" do
      ruby file: '../.ruby-version'

      gem "faraday-follow_redirects", "0.3.0"
    end

`Appraisal::BundlerDSL#ruby_version` is emitting a block when the
Gemfile DSL ruby method is used. Currently, this is emitted, which is
not a method argument but rather a block:

    ruby {:file=>"../.ruby-version"}

To fix this, we need to add `()` to ensure the arguments stay arguments:

    ruby({:file=>"../.ruby-version"})

This has been tested on Bundler 2.3.27 only — at this point in time we
have compatibility issues with others.

thoughtbot#218
@nickcharlton nickcharlton merged commit 08247f8 into thoughtbot:main Feb 15, 2024
1 check was pending
ruby({:file=>".ruby-version"})

gem "appraisal", :path => #{PROJECT_ROOT.inspect}
Gemfile
Copy link

Choose a reason for hiding this comment

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

Naming/HeredocDelimiterCase: Use uppercase heredoc delimiters.

run 'bundle install --local'
run 'appraisal generate'

expect(content_of 'gemfiles/ruby_version.gemfile').to eq <<-Gemfile.strip_heredoc
Copy link

Choose a reason for hiding this comment

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

Style/NestedParenthesizedCalls: Add parentheses to nested method call content_of 'gemfiles/ruby_version.gemfile'.
Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Metrics/LineLength: Line is too long. [85/80]

Appraisals

run 'bundle install --local'
run 'appraisal generate'
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

end
Appraisals

run 'bundle install --local'
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

appraise 'ruby-version' do
ruby file: ".ruby-version"
end
Appraisals
Copy link

Choose a reason for hiding this comment

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

Naming/HeredocDelimiterCase: Use uppercase heredoc delimiters.

ruby RUBY_VERSION

gem 'appraisal', :path => #{PROJECT_ROOT.inspect}
Gemfile
Copy link

Choose a reason for hiding this comment

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

Naming/HeredocDelimiterCase: Use uppercase heredoc delimiters.

@tisba tisba deleted the fix/ruby-hash-option branch February 15, 2024 17:12
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.

2 participants