Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch rack-accept to support all valid RFC6838 characters in media types #1179

Merged
merged 1 commit into from
Oct 9, 2015

Conversation

suan
Copy link
Contributor

@suan suan commented Oct 7, 2015

Continuation and completion of #1170

This patches rack-accept in a manner similar to how virtus was patched (as discussed)

@dblock
Copy link
Member

dblock commented Oct 7, 2015

For reference this has been PRed into rack-accept as mjackson/rack-accept#17.

@suan
Copy link
Contributor Author

suan commented Oct 7, 2015

Getting Gem::InstallError: rack-cache requires Ruby version >= 2.0.0 errors on the Rails3 Gemfile jruby build in Travis... but I don't see how these changes would cause that 😕

@dblock
Copy link
Member

dblock commented Oct 7, 2015

Maybe there was a new release of rack-cache or something like that? Would you try to hunt it down please? I'll kick it manually again see if it's intermittent.

@dblock
Copy link
Member

dblock commented Oct 7, 2015

Yep, rack-cache was released yesterday, https://rubygems.org/gems/rack-cache. So something broke there, lets fix that first?

@dblock
Copy link
Member

dblock commented Oct 7, 2015

rtomayko/rack-cache#120

@suan
Copy link
Contributor Author

suan commented Oct 7, 2015

Yeah, this seems tricky. The way I'm reading it is that Rails itself on Ruby1.9 will break now and will need to be updated, as actionpack 3.x will try to install rack-cache v1.3: https://github.com/rails/rails/blob/v3.2.22/actionpack/actionpack.gemspec

@dblock
Copy link
Member

dblock commented Oct 7, 2015

I would be ok to force an older version of rack-cache in Grape's gemfile. Want to try?

@suan
Copy link
Contributor Author

suan commented Oct 7, 2015

Sure, on it

@rnubel
Copy link
Contributor

rnubel commented Oct 7, 2015

First of all, hi @suan! So from what I can tell, only Rails 3 requires rack-cache, so I'd suggest we put this into the Appraisals file for rails-3.

@suan
Copy link
Contributor Author

suan commented Oct 7, 2015

Hey @rnubel !! 😄 So I tried putting it in Appraisals instead of the Gemfile but it doesn't seem to have the desired effect, whereas having it in the Gemfile itself worked... could u take a look and see whether my changes were what you had in mind?

@rnubel
Copy link
Contributor

rnubel commented Oct 7, 2015

@suan: you have to run appraisal install, to re-generate the gemfiles/* files. That could be part of the Travis build, maybe, but right now it's manual.

@suan
Copy link
Contributor Author

suan commented Oct 8, 2015

Thanks @rnubel - done!

@dblock Everything outside of the "Allowed Failures" section has passed. The very last specs in that section has been running for almost an hour tho and I think it's just TravisCI wonkiness?

@suan
Copy link
Contributor Author

suan commented Oct 9, 2015

Force-pushed again and Travis is 💚! :)

dblock added a commit that referenced this pull request Oct 9, 2015
Patch rack-accept to support all valid RFC6838 characters in media types
@dblock dblock merged commit 0924100 into ruby-grape:master Oct 9, 2015
@dblock
Copy link
Member

dblock commented Oct 9, 2015

Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants