De-github https filter

[rymohr]

Pretty sure this is the only filter that still includes github refs:
https://github.com/jch/html-pipeline/blob/master/lib/html/pipeline/https_filter.rb

How about piggybacking on the :base_url option instead?

[jch]

@rymohr 👍 wanna PR it?

[rymohr]

@jch btw I couldn't get the test suite to run without the following change:

-  gem.add_dependency "activesupport", RUBY_VERSION < "1.9.3" ? [">= 2", "< 4"] : ">= 2"
+  gem.add_dependency "activesupport", RUBY_VERSION < "2.1.0" ? [">= 2", "< 4"] : ">= 2"

/Users/ryan/.rbenv/versions/2.0.0-p353/lib/ruby/2.0.0/test/unit.rb:670:in `<class:Runner>': undefined

[simeonwillbanks]

The CI build errors seem to be from MiniTest.

• ruby 2.1.1
• ruby 2.0.0
• ruby 1.9.3

On CI, Bundler installs minitest 5.3.3.

If I force minitest 5.x.x, I get the same error as CI.

diff --git a/Gemfile b/Gemfile
index b985f4b..b6588ec 100644
--- a/Gemfile
+++ b/Gemfile
@@ -9,6 +9,7 @@ group :development do
 end

 group :test do
+  gem "minitest",           "~> 5"
   gem "rinku",              "~> 1.7",   :require => false
   gem "gemoji",             "~> 1.0",   :require => false
   gem "RedCloth",           "~> 4.2.9", :require => false

If I force minitest 4.7.x, I'm all green! 😄

Using minitest (4.7.5)

diff --git a/Gemfile b/Gemfile
index b985f4b..27df4be 100644
--- a/Gemfile
+++ b/Gemfile
@@ -9,6 +9,7 @@ group :development do
 end

 group :test do
+  gem "minitest",           "~> 4.7"
   gem "rinku",              "~> 1.7",   :require => false
   gem "gemoji",             "~> 1.0",   :require => false
   gem "RedCloth",           "~> 4.2.9", :require => false

@jch @rymohr Why don't we try gem "minitest", "~> 4.7" on CI? Thanks!

[rymohr]

It's the version of activesupport that's the culprit actually. If you're running the latest version of activesupport, adding minitest (~> 4.7) to the Gemfile causes this error:

Bundler could not find compatible versions for gem "minitest":
  In Gemfile:
    html-pipeline (>= 0) ruby depends on
      minitest (~> 5.1) ruby

    minitest (4.7.5)

The 4.x series of activesupport relies on minitest (~> 5.1).

[simeonwillbanks]

It looks like gem.add_dependency "activesupport", RUBY_VERSION < "1.9.3" ? [">= 2", "< 4"] : ">= 2" exists because activesupport 3.x.x requires Ruby 1.8.7 or greater and activesupport 4.x.x requires Ruby 1.9.3 or greater.

If we gem.add_dependency "activesupport", RUBY_VERSION < "2.1.0" ? [">= 2", "< 4"] : ">= 2", rubies 1.9.3 and 2.0.0 won't be able to use activesupport 4.x.x.

@rymohr However, you make a good point. Let's review activesupport releases.

• v3.2.18
  • no minitest dependency
• v4.0.0.beta1
  • 'minitest', '~> 4.2'
• v4.1.0
  • 'minitest', '~> 5.1'

Why don't we get the build passing with @rymohr's solution, and I can file an Issue to investigate a better permanent solution? After the Pull Request is merged, we can hold off on a release until the new Issue is resolved. Thoughts? Thanks!

[rymohr]

@simeonwillbanks do you know if activesupport 4.x.x would even work with ruby 2.1.0 in this case?

[simeonwillbanks]

@rymohr Since activesupport 4.1's minitest 5.1 dependency causes the failures, we can slightly refactor your solution and be green:

diff --git a/html-pipeline.gemspec b/html-pipeline.gemspec
index 9737aa6..dd625aa 100644
--- a/html-pipeline.gemspec
+++ b/html-pipeline.gemspec
@@ -16,7 +16,7 @@ Gem::Specification.new do |gem|
   gem.require_paths = ["lib"]

   gem.add_dependency "nokogiri",      RUBY_VERSION < "1.9.2" ? [">= 1.4", "< 1.6"] : "~> 1.4"
-  gem.add_dependency "activesupport", RUBY_VERSION < "1.9.3" ? [">= 2", "< 4"] : ">= 2"
+  gem.add_dependency "activesupport", RUBY_VERSION < "1.9.3" ? [">= 2", "< 4"] : [">= 2", "< 4.1"]

   gem.post_install_message = <<msg
 -------------------------------------------------

With this change, I ran the tests in all supported rubies and didn't get a failure.

This isn't awesome; html-pipeline should work with activesupport 4.1.x. 😄

[rymohr]

Thanks @simeonwillbanks. I've updated the PR with your gemspec patch. Build is still failing on REE though due to a build issue with the charlock_holmes gem.

[jch]

@simeonwillbanks 🤘 thanks for diving in and investigating.

[simeonwillbanks]

Thanks @rymohr and @jch! I've created the new issue.