This looks promising and I won't mind polishing it as patch pilot
(or helping you) but before digging I'd like some feedback from
jam and benji which have been involved recently.
[feedback] Overall, the approach sounds good. I'm not sure I'm
reading this correctly but it seems you've now made
ScopeReplacer._replace() the only place where races can occur. If
that's the case and we want to lock something, that should
probably be there.
[nit]
17 +
18 + disallow_proxying()
A bit naked ;) A comment explaining why it's done here (as early
as possible but no sooner) will be good.
[needsinfo]
218 - if event == 'line' and 'lazy_import.py' in filename:
219 - line = linecache.getline(filename, frame.f_lineno)
220 - # ...and the line of code is the one we're looking for...
221 - if 'del self._factory' in line:
<...>
228 + if (filename.endswith('lazy_import.py') and
229 + function_name == self.method_to_trace):
This looks nice and less hackish, but I thought you were relying
on the code you're replacing here in lp:~gagern/bzr/bug-702914 ?
I wouldn't mind merging these tests instead (or their moral equivalent).
Thanks for working on this !
This looks promising and I won't mind polishing it as patch pilot
(or helping you) but before digging I'd like some feedback from
jam and benji which have been involved recently.
[feedback] Overall, the approach sounds good. I'm not sure I'm _replace( ) the only place where races can occur. If
reading this correctly but it seems you've now made
ScopeReplacer.
that's the case and we want to lock something, that should
probably be there.
[nit]
17 +
18 + disallow_proxying()
A bit naked ;) A comment explaining why it's done here (as early
as possible but no sooner) will be good.
[needsinfo]
218 - if event == 'line' and 'lazy_import.py' in filename: getline( filename, frame.f_lineno) endswith( 'lazy_import. py') and to_trace) :
219 - line = linecache.
220 - # ...and the line of code is the one we're looking for...
221 - if 'del self._factory' in line:
<...>
228 + if (filename.
229 + function_name == self.method_
This looks nice and less hackish, but I thought you were relying
on the code you're replacing here in lp:~gagern/bzr/bug-702914 ?
I wouldn't mind merging these tests instead (or their moral equivalent).