Use Gem::Version to compare versions #1236
Conversation
| File.open("test/dummy/Gemfile", "w") do |gemfile| | ||
| gemfile.write('source "https://rubygems.org"') | ||
| end |
There was a problem hiding this comment.
While working in this file, if nothing was raised when a run_generator command ran, then there was a big slowdown and the following message would appear multiple times:
...[DEPRECATED] This Gemfile does not include an explicit global
source. Not using an explicit global source may result in a different
lockfile being generated depending on the gems you have
installed locally before bundler is run. Instead, define a global
source in your Gemfile like this: source "https://rubygems.org".
There was a problem hiding this comment.
Adding source "https://rubygems.org" to the Gemfile resolved this warning, making it easier to test the tests in the file.
Note that run_generator still fails for different reasons, but it's a faster fail with less noise.
| return if version.blank? | ||
|
|
||
| Gem::Version.new(version) |
There was a problem hiding this comment.
Do you think these two lines are clearer as written or combined like this?
Gem::Version.new(version) unless version.blank?
| end | ||
|
|
||
| def node_not_installed? | ||
| !node_version.present? | ||
| node_version.blank? |
There was a problem hiding this comment.
I changed this to avoid the double negative of not_installed and
!(...).present?.
louis-antonopoulos
force-pushed
the
use-gem-version-to-compare-versions
branch
6 times, most recently
from
483c690
to
b0015d3
Compare
[DEPRECATED] This Gemfile does not include an explicit global source. Not using an explicit global source may result in a different lockfile being generated depending on the gems you have installed locally before bundler is run. Instead, define a global source in your Gemfile like this: source "https://rubygems.org".
Simplify writing of source to Gemfile
louis-antonopoulos
force-pushed
the
use-gem-version-to-compare-versions
branch
from
b0015d3
to
579a24d
Compare
| version = ENV["NODE_VERSION"] || `node --version`[/\d+\.\d+\.\d+/] | ||
|
|
||
| return if version.blank? | ||
|
|
||
| Gem::Version.new(version) |
There was a problem hiding this comment.
Would the following work?
| version = ENV["NODE_VERSION"] || `node --version`[/\d+\.\d+\.\d+/] | |
| return if version.blank? | |
| Gem::Version.new(version) | |
| if version = ENV.fetch(ENV["NODE_VERSION"], `node --version`[/\d+\.\d+\.\d+/]) | |
| Gem::Version.new(version) | |
| else | |
| "" | |
| end |
It solves a couple of issues.
- Deals with
#[]returning something falsy but not nil - Gets rid of the trailing conditional and makes the method more idiomatic
- Ensures the method always returns a string
This PR:
WebGeneratorTestString comparison of version numbers can produce incorrect results. For example,
"20.10.0" < "20.2.0"resolves totrue.