Deprecate proc kwarg to call_historic()
#143
Conversation
goodboy
force-pushed
the
deprecate_proc_arg
branch
from
686c419
to
54fdbfc
Compare
| If ``proc`` is not None it will be called for for each non-None result | ||
| obtained from a hook implementation. | ||
| If ``result_callback`` is not ``None`` it will be called for for each | ||
| non-None result obtained from a hook implementation. |
There was a problem hiding this comment.
Please document that proc is deprecated in the docstring as well.
testing/test_pluginmanager.py
Outdated
| he_method1.call_historic(proc=callback, kwargs=dict(arg=1)) | ||
|
|
||
| # TODO: remove once ``proc`` is removed | ||
| if callback: |
There was a problem hiding this comment.
For completeness we should also test passing result_callback. I suggest changing this test to use result_callback and create a new, simpler test, just for the deprecation.
goodboy
force-pushed
the
deprecate_proc_arg
branch
from
54fdbfc
to
af30960
Compare
|
@nicoddemus pushed all your recs. |
pluggy/hooks.py
Outdated
| return | ||
| # XXX: woah isn't this wrong for `firstresult` hooks? |
There was a problem hiding this comment.
@nicoddemus btw did you notice this?
If i'm reading this correctly won't this code never process firstresult hooks?
There was a problem hiding this comment.
Yeah I think that loop expr is just slightly wrong should be for x in res or [res]: right?
Nope hehe.
There was a problem hiding this comment.
Oh waitttt whatttt?
Wait why not? I guess it doesn't make sense to?
pluggy/hooks.py
Outdated
| self._call_history.append((kwargs or {}, proc)) | ||
| if proc is not None: | ||
| warnings.warn( | ||
| "Support for `proc` argument now deprecated and will be" |
There was a problem hiding this comment.
Support for
procargument is now
dahhh!
goodboy
force-pushed
the
deprecate_proc_arg
branch
from
af30960
to
0333b83
Compare
It's a bad name, use `result_callback` instead. Resolves pytest-dev#120
goodboy
force-pushed
the
deprecate_proc_arg
branch
from
0333b83
to
6b296c2
Compare
|
@nicoddemus @RonnyPfannschmidt give this one more lookover if you don't mind. I tweaked couple things and added docs. |
It's a bad name, use
result_callbackinstead.Resolves #120