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

Make callstack attribute optional in AS:Deprecation #6107

Merged
merged 1 commit into from
Oct 30, 2012
Merged

Make callstack attribute optional in AS:Deprecation #6107

merged 1 commit into from
Oct 30, 2012

Conversation

gazay
Copy link
Contributor

@gazay gazay commented May 1, 2012

It's unnecessary to send caller from all places where used ActiveSupport::Deprecation.warn, because we can only call caller(2) from warn method and get the same callstack

@carlosantoniodasilva
Copy link
Member

If we're going down this path, I think we could deprecate the callstack argument.

@josevalim any concern about this?

@josevalim
Copy link
Contributor

@carlosantoniodasilva I don't think we need to deprecate, it may still be useful in some rare occasions.

@jeremy this looks good, c/d?

@carlosantoniodasilva
Copy link
Member

@josevalim alright, tks.

@steveklabnik
Copy link
Member

This will need a rebase.

@gazay
Copy link
Contributor Author

gazay commented Oct 23, 2012

@steveklabnik done. I've added optional callstack functionality to new method too - deprecation_warning of ActiveSupport::Deprecation::Reporting. And entry to Changelog.

@carlosantoniodasilva
Copy link
Member

@gazay now this needs a new rebase :). You can also extend it and take a look at some other places where caller is given, to remove it (like some of the places from the other pull request you commented).

@gazay
Copy link
Contributor Author

gazay commented Oct 30, 2012

Yep, I'm on it right now.

@gazay
Copy link
Contributor Author

gazay commented Oct 30, 2012

@carlosantoniodasilva done

@carlosantoniodasilva
Copy link
Member

@gazay Nice, thank you.

carlosantoniodasilva added a commit that referenced this pull request Oct 30, 2012
…ion_optional

Make callstack attribute optional in AS:Deprecation
@carlosantoniodasilva carlosantoniodasilva merged commit 0ad6c08 into rails:master Oct 30, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants