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 deprecation warning message #18923

Merged
merged 2 commits into from
Feb 13, 2015

Conversation

tgxworld
Copy link
Contributor

No description provided.

@@ -306,7 +306,8 @@ def kwarg_request?(*args)
def non_kwarg_request_warning
ActiveSupport::Deprecation.warn(<<-MSG.strip_heredoc)
ActionDispatch::Integration::TestCase HTTP request methods will accept only
keyword arguments in future Rails versions.
the following keyword arguments in future Rails versions:
#{REQUEST_KWARGS.join(', ')}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was doing something like post '/documents', document: { title: "New things", content: "Doing them" } and saw the deprecation message.

It wasn't immediately clear to me what the keyword arguments are.

kaspth added a commit that referenced this pull request Feb 13, 2015
@kaspth kaspth merged commit 7ce3174 into rails:master Feb 13, 2015
@kaspth
Copy link
Contributor

kaspth commented Feb 13, 2015

Thanks 💛

@tgxworld
Copy link
Contributor Author

@kaspth You're welcome 😄

@@ -314,9 +315,7 @@ def non_kwarg_request_warning
params: { id: 1 },
headers: { 'X-Extra-Header' => '123' },
env: { 'action_dispatch.custom' => 'custom' }
Copy link
Contributor

Choose a reason for hiding this comment

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

@tgxworld missed adding a trailing coma?

Copy link
Member

Choose a reason for hiding this comment

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

good catch

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

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.

None yet

4 participants