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

Replace Protobuf::Deprecator with a thin wrapper on AS::Deprecation #232

Merged
merged 8 commits into from
Dec 2, 2014
Merged

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Nov 24, 2014

@tamird
Copy link
Contributor Author

tamird commented Dec 1, 2014

Did a bit more work on this - ended up replacing our bespoke deprecator module with a subclass of ActiveSupport::Deprecation which makes things generally much thinner.

@tamird
Copy link
Contributor Author

tamird commented Dec 1, 2014

cc @liveh2o @abrandoned

end
end
end
::Protobuf.deprecator.define_deprecated_methods(Numeric, :value => :to_int)
Copy link
Contributor

Choose a reason for hiding this comment

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

So much win.

@localshred
Copy link
Contributor

Looks good to me :shipit:

Will review other PRs tonight and figure out the order we want all this merged (if order indeed will matter).

@tamird
Copy link
Contributor Author

tamird commented Dec 2, 2014

order probably matters, but there's likely no order that is rebase-free. if this one LGTY, go ahead and merge it, and i'll rebase the others

@tamird tamird changed the title Remove uses of class_eval Replace Protobuf::Deprecator with a thin wrapper on AS::Deprecation Dec 2, 2014
localshred added a commit that referenced this pull request Dec 2, 2014
Replace Protobuf::Deprecator with a thin wrapper on AS::Deprecation
@localshred localshred merged commit 604feaf into ruby-protobuf:master Dec 2, 2014
@tamird tamird deleted the class_eval branch December 2, 2014 07:54
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.

2 participants