Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Commented metaprogramming turned out to be noisier not clearer

  • Loading branch information...
commit 6b12d74026808a3014f1dff34481006a96e0f18f 1 parent bf0f145
@jeremy jeremy authored
View
6 activesupport/lib/active_support/core_ext/proc.rb
@@ -5,9 +5,9 @@ def bind(object)
block, time = self, Time.now
object.class_eval do
method_name = "__bind_#{time.to_i}_#{time.usec}"
- define_method(method_name, &block) # define_method("__bind_1230458026_720454", &block)
- method = instance_method(method_name) # method = instance_method("__bind_1230458026_720454")
- remove_method(method_name) # remove_method("__bind_1230458026_720454")
+ define_method(method_name, &block)
+ method = instance_method(method_name)
+ remove_method(method_name)
method
end.bind(object)
end
View
18 activesupport/lib/active_support/deprecation/method_wrappers.rb
@@ -12,15 +12,15 @@ def deprecate_methods(target_module, *method_names)
method_names.each do |method_name|
target_module.alias_method_chain(method_name, :deprecation) do |target, punctuation|
target_module.module_eval(<<-end_eval, __FILE__, __LINE__ + 1)
- def #{target}_with_deprecation#{punctuation}(*args, &block) # def generate_secret_with_deprecation(*args, &block)
- ::ActiveSupport::Deprecation.warn( # ::ActiveSupport::Deprecation.warn(
- ::ActiveSupport::Deprecation.deprecated_method_warning( # ::ActiveSupport::Deprecation.deprecated_method_warning(
- :#{method_name}, # :generate_secret,
- #{options[method_name].inspect}), # "You should use ActiveSupport::SecureRandom.hex(64)"),
- caller # caller
- ) # )
- send(:#{target}_without_deprecation#{punctuation}, *args, &block) # send(:generate_secret_without_deprecation, *args, &block)
- end # end
+ def #{target}_with_deprecation#{punctuation}(*args, &block)
+ ::ActiveSupport::Deprecation.warn(
+ ::ActiveSupport::Deprecation.deprecated_method_warning(
+ :#{method_name},
+ #{options[method_name].inspect}),
+ caller
+ )
+ send(:#{target}_without_deprecation#{punctuation}, *args, &block)
+ end
end_eval
end
end

5 comments on commit 6b12d74

@iain

is this a consensus with the Rails team, or just in these two cases?

@josh
Collaborator

I'm in favor of removing them all. They detract value.

@radar

If you want to understand what these methods do, the guides should cover them.

@iain

@josh I don't know what that really means "they detract value". Which value? How?

I might be tempted to say that if these comments are really necessary, the code might be a bit too complex. These ones are alright, but I remember some routing code in which these comments really helped.

@jeremy
Owner

The comments distracted from the code rather than clarifying it. I only happened to hit these two cases but there's likely more worth reverting should anyone feel patch-inclined.

Please sign in to comment.
Something went wrong with that request. Please try again.