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

Improve Errors when Controller Name or Action isn't specfied #12740

Merged
merged 1 commit into from
Nov 11, 2013
Merged

Improve Errors when Controller Name or Action isn't specfied #12740

merged 1 commit into from
Nov 11, 2013

Conversation

gaurish
Copy link
Contributor

@gaurish gaurish commented Nov 2, 2013

These errors occur when, there routes are wrongly defined.

example, the following lines would cause a missing :action error

root "welcomeindex"
root "welcome/index"

Mostly beginners are expected to hit these errors, so lets improve the error message a bit to make their learning experience bit better.

@robin850
Copy link
Member

robin850 commented Nov 2, 2013

Hello,

Thanks for your patch! 👍 Maybe we could add a test case for this ? Also, can you add a changelog entry please ?

@arunagw
Copy link
Member

arunagw commented Nov 2, 2013

I don't like the idea of giving links for guides for these errors.

Also these guides are dependent on versions. So in every release we need to change URLS here?

@gaurish
Copy link
Contributor Author

gaurish commented Nov 2, 2013

@arunagw
PR updated with suggestions

FYI, I added links after looking at this code. Thinking giving links for guides for these errors is a practice.

@robin850
no code/behaviour has been affected by this. Do you think does this small documentation patch needs a testcase or mention in changelog?

@vipulnsward
Copy link
Member

I dont think this needs a changelog/test.

On Saturday, November 2, 2013, Gaurish Sharma wrote:

@arunagw https://github.com/arunagw
Makes sense. Having to update guides link on every release is bad.

I added links after looking at this codehttps://github.com/rails/rails/blob/masteractionpack/lib/action_dispatch/routing/mapper.rb#L240-L244.
Thinking giving links for guides for these errors is a practice. But I have
removed it now.

@robin850 https://github.com/robin850
no code/behaviour has been affected by this. Do you think does this small
documentation patch needs a testcase or mention in changelog?


Reply to this email directly or view it on GitHubhttps://github.com//pull/12740#issuecomment-27620512
.

Vipul A.M.
+91-8149-204995

end

if action.blank? && segment_keys.exclude?(:action)
raise ArgumentError, "missing :action"
message = "Error missing :action key from params. possibly, action isn't correctly specified. check your routes"
raise ArgumentError, message
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make the messages look like the same? How about this:

Missing :controller key on routes definition, please check your routes.

@robin850
Copy link
Member

robin850 commented Nov 2, 2013

Yes, you are right guys, sorry!

These errors occur when, there routes are wrongly defined.

example, the following line would cause a missing :action error

    root "welcomeindex"

Mostly beginners are expected to hit these errors, so lets improve the error message a bit to make their learning experience bit better.
@gaurish
Copy link
Contributor Author

gaurish commented Nov 5, 2013

@carlosantoniodasilva
Done. made the change. squashed/rebased

@dmathieu
Copy link
Contributor

dmathieu commented Nov 5, 2013

Does this really needs to assign variables?

@gaurish
Copy link
Contributor Author

gaurish commented Nov 5, 2013

@dmathieu Not really. use of variable make this code easy to read & also this pattern is used throughout the file. Any reason NOT to use variables here?

rafaelfranca added a commit that referenced this pull request Nov 11, 2013
Improve Errors when Controller Name or Action isn't specfied
@rafaelfranca rafaelfranca merged commit 4a344c5 into rails:master Nov 11, 2013
rafaelfranca added a commit that referenced this pull request Nov 11, 2013
Improve Errors when Controller Name or Action isn't specfied
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.

7 participants