Use Addressable::URI, allowing IPv6 URLs #339
Conversation
"Addressable is a replacement for the URI implementation that is part of Ruby's standard library. It more closely conforms to RFC 3986, RFC 3987, and RFC 6570 (level 4), additionally providing support for IRIs and URI templates.", see https://github.com/sporkmonger/addressable Fixes inspec#110 Signed-off-by: Markus Grobelin <grobi@koppzu.de>
|
I didn't add |
|
Thanks for this @mgrobelin ! I would go ahead and add the gem as a dependency for both the gemfile and gemspec incase something else is removed that is depending on it. |
lib/train.rb
Outdated
| string += '//dummy' | ||
| else | ||
| raise Train::UserError, e | ||
| if u.scheme and (u.host.nil? or u.host.empty?) and u.path.empty? |
There was a problem hiding this comment.
Our standard is to use the || and &&, instead of and/or due to the precedence differences.
Signed-off-by: Markus Grobelin <grobi@koppzu.de>
mgrobelin
force-pushed
the
addressable_gem_instead_of_stdlib_uri
branch
from
94659ad
to
5810940
Compare
clintoncwolfe
changed the title
There was a problem hiding this comment.
This looks great! A few simple changes...
- Please remove addressable from the Gemfile
- You have a couple of linting failures. Run 'bundle exec rubocop' to see them - any linting failures will fail the tests, which fails CI, which means we can't accept it yet.
Thanks so much!
Gemfile
Outdated
| @@ -12,6 +12,8 @@ if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.2') | |||
| gem 'json', '< 2.0' | |||
| end | |||
|
|
|||
| gem 'addressable', '~> 2.5' | |||
There was a problem hiding this comment.
This isn't a development need - it should only be in the gemspec. Gemfile pulls in all gemspec requirements, so this is redundant.
see inspec#339 (review) Signed-off-by: Markus Grobelin <grobi@koppzu.de>
| @@ -25,6 +25,7 @@ Gem::Specification.new do |spec| | |||
|
|
|||
| spec.required_ruby_version = '>= 2.0' | |||
|
|
|||
| spec.add_dependency 'addressable', '~> 2.5' | |||
There was a problem hiding this comment.
The pin here on 2.5 is going to conflict with Chef which is pinned to 2.4 due to another gem. Does this need to be 2.5 or would 2.4 work?
There was a problem hiding this comment.
Hmm, addressable 2.4 is from end of 2015. Maybe I was required to bump the gem while working on a project for Ruby 2.5 support, but maybe I remember wrong.
For sure a lot of fixes happened since 2.4. I don't know whether the change to addressable gem is even smart when not used with 2.5
There was a problem hiding this comment.
We're currently waiting on Travis to release this gem so we can pull in the latest addressable to our overall stack: travis-ci/gh#33
clintoncwolfe
added
Incomplete/Linting
Type: Enhancement
see #339 (review) Signed-off-by: Markus Grobelin <grobi@koppzu.de>
see #339 (review) Signed-off-by: Markus Grobelin <grobi@koppzu.de>
|
Closing in favor of #566, which I'll try to merge soon. |
"Addressable is a replacement for the URI implementation that is part of Ruby's standard library. It more closely conforms to RFC 3986, RFC 3987, and RFC 6570 (level 4), additionally providing support for IRIs and URI templates.", see https://github.com/sporkmonger/addressable
Fixes #110
Signed-off-by: Markus Grobelin grobi@koppzu.de