Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

fix bug that causes "run rake false" to appear in README.md #3881

Merged
merged 4 commits into from
Jul 27, 2015

Conversation

roseweixel
Copy link
Contributor

If BUNDLE_GEM__TEST: false or BUNDLE_GEM__TEST: "false" are in ~/.bundle/config, the README.md that is generated when creating a new gem includes instructions under the Development heading that say "Then, run rake false to run the tests."

The logic on line 29 of README.md.tt (if config[:test]) fails because config[:test] returns the string "false" instead of the boolean.

As a patch, I changed the logic of Bundler::Settings#[] to convert the string "false" to a boolean.

It seems that the root of the issue is that Bundler::Settings::BOOL_KEYS does not include "gem.test", and it can't because it is only sometimes a boolean (when it is not 'rspec' or 'minitest'). A future fix for this could be to have two separate options, a boolean for whether or not the gem has tests and another for the type of test framework.

@indirect
Copy link
Member

This is a great patch, thanks! 😁 If you're interested in contributing more to Bundler in the future, drop me an email, and I'll invite you to the Bundler Slack!

@indirect
Copy link
Member

@homu r+

@homu
Copy link
Contributor

homu commented Jul 27, 2015

📌 Commit ed59ad7 has been approved by indirect

@homu
Copy link
Contributor

homu commented Jul 27, 2015

⌛ Testing commit ed59ad7 with merge 5b92fa4...

homu added a commit that referenced this pull request Jul 27, 2015
fix bug that causes "run `rake false`" to appear in README.md

If `BUNDLE_GEM__TEST: false` or `BUNDLE_GEM__TEST: "false"` are in `~/.bundle/config`, the README.md that is generated when creating a new gem includes instructions under the Development heading that say "Then, run \`rake false\` to run the tests."

The logic on line 29 of  README.md.tt (`if config[:test]`) fails because `config[:test]` returns the string "false" instead of the boolean.

As a patch, I changed the logic of `Bundler::Settings#[]` to convert the string "false" to a boolean.

It seems that the root of the issue is that `Bundler::Settings::BOOL_KEYS` does not include "gem.test", and it can't because it is only sometimes a boolean (when it is not 'rspec' or 'minitest'). A future fix for this could be to have two separate options, a boolean for whether or not the gem has tests and another for the type of test framework.
@homu
Copy link
Contributor

homu commented Jul 27, 2015

💔 Test failed - status

when !value.nil? && is_bool(name)
when value.nil?
nil
when is_bool(name) || value == "false"
Copy link
Member

Choose a reason for hiding this comment

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

trailing whitespace here

@roseweixel
Copy link
Contributor Author

bc94ea0 fixes the whitespace issue - should be ok now!

@segiddins
Copy link
Member

@homu r+

@homu
Copy link
Contributor

homu commented Jul 27, 2015

📌 Commit bc94ea0 has been approved by segiddins

@homu
Copy link
Contributor

homu commented Jul 27, 2015

⌛ Testing commit bc94ea0 with merge 6171e73...

homu added a commit that referenced this pull request Jul 27, 2015
fix bug that causes "run `rake false`" to appear in README.md

If `BUNDLE_GEM__TEST: false` or `BUNDLE_GEM__TEST: "false"` are in `~/.bundle/config`, the README.md that is generated when creating a new gem includes instructions under the Development heading that say "Then, run \`rake false\` to run the tests."

The logic on line 29 of  README.md.tt (`if config[:test]`) fails because `config[:test]` returns the string "false" instead of the boolean.

As a patch, I changed the logic of `Bundler::Settings#[]` to convert the string "false" to a boolean.

It seems that the root of the issue is that `Bundler::Settings::BOOL_KEYS` does not include "gem.test", and it can't because it is only sometimes a boolean (when it is not 'rspec' or 'minitest'). A future fix for this could be to have two separate options, a boolean for whether or not the gem has tests and another for the type of test framework.
@homu
Copy link
Contributor

homu commented Jul 27, 2015

💔 Test failed - status

@segiddins
Copy link
Member

@homu retry

@homu
Copy link
Contributor

homu commented Jul 27, 2015

⌛ Testing commit bc94ea0 with merge 2fe783b...

homu added a commit that referenced this pull request Jul 27, 2015
fix bug that causes "run `rake false`" to appear in README.md

If `BUNDLE_GEM__TEST: false` or `BUNDLE_GEM__TEST: "false"` are in `~/.bundle/config`, the README.md that is generated when creating a new gem includes instructions under the Development heading that say "Then, run \`rake false\` to run the tests."

The logic on line 29 of  README.md.tt (`if config[:test]`) fails because `config[:test]` returns the string "false" instead of the boolean.

As a patch, I changed the logic of `Bundler::Settings#[]` to convert the string "false" to a boolean.

It seems that the root of the issue is that `Bundler::Settings::BOOL_KEYS` does not include "gem.test", and it can't because it is only sometimes a boolean (when it is not 'rspec' or 'minitest'). A future fix for this could be to have two separate options, a boolean for whether or not the gem has tests and another for the type of test framework.
@homu
Copy link
Contributor

homu commented Jul 27, 2015

💔 Test failed - status

@indirect
Copy link
Member

Okay, I'm tired of non-deterministic failures. Merging! Thanks again for the fix.

indirect added a commit that referenced this pull request Jul 27, 2015
fix bug that causes "run `rake false`" to appear in README.md
@indirect indirect merged commit 0433b83 into rubygems:master Jul 27, 2015
@roseweixel
Copy link
Contributor Author

Thanks for merging, happy to contribute!

@coilysiren coilysiren added this to the Release Archive milestone Oct 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants