Bug #7815
closedBackport: Warning about TracePoint events to 2.0.0
Description
Can we please backport r39168 to 2_0_0 branch?
https://github.com/ruby/ruby/commit/810516e
and
https://github.com/ruby/ruby/commit/810516e.patch
Updated by ko1 (Koichi Sasada) over 11 years ago
mame-san:
This additional document is important to avoid future compatibility issue.
* *Note* do not depend on current event set, as this list is subject to
* change. Instead, it is recommended you specify the type of events you
* want to use.
Please see a problem of `set_trace_func'
http://www.atdot.net/~ko1/diary/201212.html#d12
(sorry, it is written in Japanese).
Updated by mame (Yusuke Endoh) over 11 years ago
Almost okay (because of only rdoc fix) but the line may matter:
- Note: this method is obsolete, please use TracePoint instead.
Suddenly making it obsolete is not a good idea after rc2, I think.
Or, is it really discussed and decided by matz?
In addition, Thread#set_trace_func should not refer the obsolete method:
- See Kernel#set_trace_func.
--
Yusuke Endoh mame@tsg.ne.jp
Updated by zzak (zzak _) over 11 years ago
Thank you for the review Yusuke-san!
In my opinion Kernel#set_trace_func is obsolete, because it's an older and outdated API for TracePoint. We should recommend users try TracePoint instead, but it's not an officially deprecated feature. Ok?
In addition, Thread#set_trace_func should not refer the obsolete method
There is a bug in TracePoint spec where you cannot specify listen for a target thread, so Thread#set_trace_func is still acceptable API and preferred method for doing this. In this case, referring to Kernel#set_trace_func still applies because it carries additional api on this API. Until TracePoint spec is fixed, we can still cross-reference #set_trace_func.
If you'd like I can supply a patch only for warning on TracePoint events, it's up to you.
Updated by mame (Yusuke Endoh) over 11 years ago
zzak (Zachary Scott) wrote:
In my opinion Kernel#set_trace_func is obsolete, because it's an older and outdated API for TracePoint. We should recommend users try TracePoint instead, but it's not an officially deprecated feature. Ok?
I don't know. It is officially deprecated only if matz approve the deprecation. Ko1, do you know?
Also, I'm afraid if TracePoint is not mature enough to deprecate Kernel#set_trace_func immediately.
So I think it would be good to include both in 2.0.0.
In addition, Thread#set_trace_func should not refer the obsolete method
There is a bug in TracePoint spec where you cannot specify listen for a target thread, so Thread#set_trace_func is still acceptable API and preferred method for doing this. In this case, referring to Kernel#set_trace_func still applies because it carries additional api on this API. Until TracePoint spec is fixed, we can still cross-reference #set_trace_func.
Yes I know. My opinion is just a matter of form; it looks strange to me that the rdoc of a method that is not deprecated yet depends on a deprecated method's.
If you'd like I can supply a patch only for warning on TracePoint events, it's up to you.
I prefer this, unless matz officially deprecated only Kernel#set_trace_func. Thanks!
--
Yusuke Endoh mame@tsg.ne.jp
Updated by ko1 (Koichi Sasada) over 11 years ago
(2013/02/10 10:19), mame (Yusuke Endoh) wrote:
If you'd like I can supply a patch only for warning on TracePoint events, it's up to you.
I prefer this, unless matz officially deprecated only Kernel#set_trace_func. Thanks!
+1
One more point:
zzak uses obsolete' and mame uses
deprecated'.
zzak wants to say recommend to use TracePoint' by the word
obsolete'.
But people think this word as `deprecated' like mame-san.
--
// SASADA Koichi at atdot dot net
Updated by zzak (zzak _) over 11 years ago
- Status changed from Open to Assigned
It is as Koichi-san says, I don't mean to deprecate Kernel#set_trace_func. Only to advise users to try TracePoint for new programs, since #set_trace_func is the old API.
Anyways, here's the patch for only the warning, I'm not sure how to commit to separate branches in svn. Yusuke-san, do you think you can do this for me?
https://github.com/zzak/ruby/commit/4bc46c4.patch
Thanks!
Updated by mame (Yusuke Endoh) over 11 years ago
- Assignee changed from mame (Yusuke Endoh) to zzak (zzak _)
Looks good to me. Go ahead. Thank you!
--
Yusuke Endoh mame@tsg.ne.jp
Updated by zzak (zzak _) over 11 years ago
- Status changed from Assigned to Closed
Resolved by r39237