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

Fix incorrect namespace of #named_routes. #1481

Merged
merged 2 commits into from Nov 4, 2013

Conversation

Projects
None yet
2 participants
@namusyaka
Member

namusyaka commented Oct 30, 2013

ref #1095
I fixed #named_routes behavior.

BTW, should we fix url behavior?
If so, I'm going to modify the behavior in this way.

url(:cap_alerts, :index) #=> "/cap_alerts"
url(:cap, :alerts, :index) #=> UnrecognizedException
url(:cap, :alerts_index) #=> UnrecognizedException
@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Oct 30, 2013

Member

url(:cap, :alerts, :index) looks weird. Does it conform with anything?

Member

ujifgc commented Oct 30, 2013

url(:cap, :alerts, :index) looks weird. Does it conform with anything?

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka Oct 30, 2013

Member

Yeah, however that code works on current.
Because routing saves route.name that includes controller name as snake case.
And, "_" joins the arguments in url.
Should I fix this?

Member

namusyaka commented Oct 30, 2013

Yeah, however that code works on current.
Because routing saves route.name that includes controller name as snake case.
And, "_" joins the arguments in url.
Should I fix this?

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka Oct 30, 2013

Member

Well, if you permit to change route.name (:controller_name => :"controller name"), I can fix it.
If not so, I'm not sure.

Member

namusyaka commented Oct 30, 2013

Well, if you permit to change route.name (:controller_name => :"controller name"), I can fix it.
If not so, I'm not sure.

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Oct 30, 2013

Member

Yeah, I think we should exclude that strange syntax. I don't think that is intentional behavior.

Member

ujifgc commented Oct 30, 2013

Yeah, I think we should exclude that strange syntax. I don't think that is intentional behavior.

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka Oct 30, 2013

Member

Okay, I'm done!
This change is big. Please reviewing.

before

url(:cap_alerts, :index)  #=> "/cap_alerts"
url(:cap, :alerts, :index) #=> "/cap_alerts"
url(:cap, :alerts_index)  #=> "/cap_alerts"

after

url(:cap_alerts, :index)  #=> "/cap_alerts"
url(:cap, :alerts, :index) #=> UnrecognizedException
url(:cap, :alerts_index)  #=> UnrecognizedException

And, will change the return value of route.name.

before

route.name #=> :cap_alerts_index

after

route.name #=> :"cap_alerts index"
Member

namusyaka commented Oct 30, 2013

Okay, I'm done!
This change is big. Please reviewing.

before

url(:cap_alerts, :index)  #=> "/cap_alerts"
url(:cap, :alerts, :index) #=> "/cap_alerts"
url(:cap, :alerts_index)  #=> "/cap_alerts"

after

url(:cap_alerts, :index)  #=> "/cap_alerts"
url(:cap, :alerts, :index) #=> UnrecognizedException
url(:cap, :alerts_index)  #=> UnrecognizedException

And, will change the return value of route.name.

before

route.name #=> :cap_alerts_index

after

route.name #=> :"cap_alerts index"
@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc
Member

ujifgc commented Oct 30, 2013

ref #1464

ujifgc added a commit that referenced this pull request Nov 4, 2013

Merge pull request #1481 from padrino/fix-named-routes
Fix incorrect namespace of #named_routes. Fixes #1095

@ujifgc ujifgc merged commit cbb0026 into master Nov 4, 2013

1 check passed

default The Travis CI build passed
Details

@ujifgc ujifgc deleted the fix-named-routes branch Nov 4, 2013

namusyaka added a commit to namusyaka/rspec-padrino that referenced this pull request Mar 30, 2014

Padrino 0.12.0 is available
Update the code about named_routes
named_route was modified in padrino/padrino-framework#1481
and padrino/padrino-framework@9ee1143

namusyaka added a commit to namusyaka/rspec-padrino that referenced this pull request Mar 30, 2014

Padrino 0.12.0 is available
Update the code about named_routes
named_route was modified in padrino/padrino-framework#1481
and padrino/padrino-framework@9ee1143
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment