Closed AvnerCohen closed 8 years ago
Hi Avner,
Moving the shortener list to an external file to ease maintenance sounds like a good idea, particularly in light of #2.
Maybe it would be best to first have a commit that takes the existing list of shorteners and loads them from a text file and then a single commit with your changes to the list?
As for how to load the shorteners, there is the following example in the README:
ShortenerList.new(File.readlines('shorteners.txt').map(&:chomp))
But we'll likely need to drop the Symbol#to_proc
usage for Ruby 1.8.7 compatibility so perhaps something like the following in lib/embiggen/configuration.rb
:
hosts = File.readlines(File.expand_path('../../../shorteners.txt', __FILE__))
hosts.each { |host| host.chomp! }
@shorteners ||= ShortenerList.new(hosts)
Which would load the shorteners from shorteners.txt
in the root directory (which we'd need to add to the gemspec).
You intend to support Ruby 1.8.7 going forward, it's EOL was announced 2.5 years ago: https://www.ruby-lang.org/en/news/2013/06/30/we-retire-1-8-7/
As for the approach itself, sounds good, I'll take a stub at this in the next few days.
Yeah: as Embiggen is a library, I believe it should be compatible with as many Ruby versions as is reasonable (taking RSpec's example) even if very few people would ever write a new app using 1.8.
As we don't rely on any newer features of Ruby or have any dependencies that make it difficult, it seems like the effort is worth it for now.
Added shoreteners: bc.vc adfoc.us bit.do seomafia.net fur.ly Removed shoreteners: stub_redirects.ca
As a side, I felt the list in the code is a a bit too hard to work with (sort, check if present, read through), would you take a separate PR that switches that to read the content from a local txt/cfg file?
This will also allow sending a filename into the gem such that it's possible to configure a clean or extended list from external code.
Thoughts?