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

Fix method rename in the latest versions of airbrake - Closes #444 #449

Closed
wants to merge 1 commit into from
Closed

Fix method rename in the latest versions of airbrake - Closes #444 #449

wants to merge 1 commit into from

Conversation

dmathieu
Copy link
Contributor

@dmathieu dmathieu commented Apr 4, 2013

Since airbrake/airbrake@8768b1c, BacktraceLine#method has been renamed into BacktraceLine#method_name, therefore breaking errbit for people using the latest versions of the notifier.

This fixes it.

@shingara
Copy link
Member

shingara commented Apr 4, 2013

Why not just alias method_name= by method=

alias :method_name= :method=

I think it's enought to work and didn't break compatibility. Because this commit break all old airbrake gem and delete all error doing this this commit.

I did not agree to this Pull request.

@dmathieu
Copy link
Contributor Author

dmathieu commented Apr 4, 2013

The usage of method is not a good idea, as the method is already implemented by ruby.
I don't mind aliasing :method to :method_name, for old notifiers compatibility.
But as it's clearly not a good idea to keep it in the long term, we shouldn't just alias.

@shingara
Copy link
Member

shingara commented Apr 4, 2013

agree about the long term. But we need alias :method= and :method_name= in your commit and do a migration to be a good PR.

@ndbroadbent
Copy link
Member

I agree that alias :method= :method_name= is necessary, otherwise error reports from the old notifier will cause an error like: Undefined method 'method=' for BacktraceLine

@ndbroadbent
Copy link
Member

We will also need to update app/models/backtrace_line_normalizer.rb, lib/tasks/errbit/demo.rake and lib/tasks/errbit/database.rake

@swibs
Copy link

swibs commented Apr 11, 2013

@dmathieu Why was this pull request closed?

@dmathieu
Copy link
Contributor Author

Because I don't have enough time to work on it and I don't want anybody to be prevented to fix this because there's something open.

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