This repository has been archived by the owner. It is now read-only.

Rails 4 route mapper compatibility #24

Merged
merged 2 commits into from Jan 9, 2013

Conversation

Projects
None yet
4 participants
@nilbus
Collaborator

nilbus commented Jan 9, 2013

@jamesotron

This comment has been minimized.

Show comment
Hide comment
@jamesotron

jamesotron Jan 9, 2013

Owner

This is great. Do we need to match against the other HTTP verbs though?

Owner

jamesotron commented Jan 9, 2013

This is great. Do we need to match against the other HTTP verbs though?

@nilbus

This comment has been minimized.

Show comment
Hide comment
@nilbus

nilbus Jan 9, 2013

Collaborator

I was wondering that - what do you think? I can't imagine using other verbs to connect from a client, but I figured you would know better. Technically to maintain full backward compatibility, it should include all verbs. How far do you take it? I'm open to your thoughts on this though.

Collaborator

nilbus commented Jan 9, 2013

I was wondering that - what do you think? I can't imagine using other verbs to connect from a client, but I figured you would know better. Technically to maintain full backward compatibility, it should include all verbs. How far do you take it? I'm open to your thoughts on this though.

@jamesotron

This comment has been minimized.

Show comment
Hide comment
@nilbus

This comment has been minimized.

Show comment
Hide comment
@nilbus

nilbus Jan 9, 2013

Collaborator

On second glance at the Rails 4 code, it looks like you can use match ..., via: :all. And given that post, I think we should. I'll test and modify the pull request.

Collaborator

nilbus commented Jan 9, 2013

On second glance at the Rails 4 code, it looks like you can use match ..., via: :all. And given that post, I think we should. I'll test and modify the pull request.

@jamesotron

This comment has been minimized.

Show comment
Hide comment
@jamesotron

jamesotron Jan 9, 2013

Owner

Cool, thanks!

Owner

jamesotron commented Jan 9, 2013

Cool, thanks!

@nilbus

This comment has been minimized.

Show comment
Hide comment
@nilbus

nilbus Jan 9, 2013

Collaborator

This works with Rails 4 and matches all HTTP verbs like before. Good catch!

Collaborator

nilbus commented Jan 9, 2013

This works with Rails 4 and matches all HTTP verbs like before. Good catch!

jamesotron pushed a commit that referenced this pull request Jan 9, 2013

James Harton
Merge pull request #24 from nilbus/rails4
Rails 4 route mapper compatibility

@jamesotron jamesotron merged commit 7263922 into jamesotron:master Jan 9, 2013

1 check passed

default The Travis build passed
Details
@jamesotron

This comment has been minimized.

Show comment
Hide comment
@jamesotron

jamesotron Jan 9, 2013

Owner

Thanks for the PR.

Owner

jamesotron commented Jan 9, 2013

Thanks for the PR.

@nilbus

This comment has been minimized.

Show comment
Hide comment
@nilbus

nilbus Jan 9, 2013

Collaborator

And you for the quick merge! I'll let you know if I hit any other issues with Rails 4 compatibility.

Collaborator

nilbus commented Jan 9, 2013

And you for the quick merge! I'll let you know if I hit any other issues with Rails 4 compatibility.

jamesotron added a commit that referenced this pull request Feb 5, 2013

@aratak

This comment has been minimized.

Show comment
Hide comment
@aratak

aratak Mar 11, 2013

Guys, am I correctly understand that via: :all doesn't work with old rails?
In such event can we use via: [:get, :post]?

aratak commented Mar 11, 2013

Guys, am I correctly understand that via: :all doesn't work with old rails?
In such event can we use via: [:get, :post]?

@jamesotron

This comment has been minimized.

Show comment
Hide comment
@jamesotron

jamesotron Mar 11, 2013

Owner

This is something we're going to have to fix soon anyway, since we're so close to a Rails 4 release. We're going to need to increase the test coverage a bit so that we can prove that it works on Rails 3.x and Rails 4.

Owner

jamesotron commented Mar 11, 2013

This is something we're going to have to fix soon anyway, since we're so close to a Rails 4 release. We're going to need to increase the test coverage a bit so that we can prove that it works on Rails 3.x and Rails 4.

@aaronjensen

This comment has been minimized.

Show comment
Hide comment
@aaronjensen

aaronjensen Apr 28, 2013

Collaborator

This should be reopened, yea?

Collaborator

aaronjensen commented Apr 28, 2013

This should be reopened, yea?

@nilbus

This comment has been minimized.

Show comment
Hide comment
@nilbus

nilbus Apr 28, 2013

Collaborator

Yes

Collaborator

nilbus commented Apr 28, 2013

Yes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.