Made changes for easier ruby 1.8 integration #384

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

joshreed commented Jun 19, 2012

Hi, I needed to make these changes for a project I'm working on (a rails 3.0.3 project that had some strong gem dependencies that didn't allow me to update rack to version >= 1.4). And the entire project was created for ruby 1.8.7, and upgrading to 1.9+ is not an option at this point--though it is planned for the future when time permits.

I feel that this situation may be somewhat common, and therefore these changes allow Tire to work properly without as many dependency headaches. It's up to you if you want to include these changes, but by making these changes it certainly made my life easier!

Owner

karmi commented Jul 26, 2012

@joshreed So, basically, you're suggesting to drop the rack/backports/uri/common_18 patch from Rack (so we don't have to require the 1.4 version) and drop it in directly? I'm not against it, if that solves the problem, just wanna make sure I understand the picture...

Contributor

joshreed commented Jul 26, 2012

@karmi Yep, that's exactly correct. Ultimately it creates less gem dependencies that could be annoying for those who have code still using ruby 1.8.7. Like I said, it's completely up to you if you want to include it, it just solved my problem and I thought it could be helpful for others as well.

Even though it could be considered slightly on the non-DRY side, to me the decrease in dependencies is worth it. Plus, since it is a back-port of ruby 1.9 functionality anyway, it doesn't seem bad to me for Tire to include its own copy of the necessary back-ported functionality.

I only tested it with Rack 1.2+, and I didn't completely go through the Tire code to see if you use Rack for anything else (besides this back-ported functionality), but it may be possible to completely drop the dependency on Rack by making this change, which could be even better.

Anyway, let me know if you have any other questions, I'd be glad to help.

karmi closed this in c772948 Oct 18, 2012

Owner

karmi commented Oct 18, 2012

OK, merged, fixed, pushed. Rack wasn't indeed used for anything else, just the URI escaping.

@NOX73 NOX73 added a commit to NOX73/tire that referenced this pull request Oct 23, 2012

@joshreed @NOX73 joshreed + NOX73 Changed the URI escape/unescape compatibility patch to not require Rack
Originally by @joshreed, cleanup by @karmi.

Closes #384.
8a56d63
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment