Some outstanding docs #85
Conversation
| By default hooks are :ref:`called <calling>` in LIFO registered order, however, | ||
| a *hookimpl* can influence its call-time invocation position using special | ||
| attributes. If marked with a ``"tryfirst"`` or ``"trylast"`` option it | ||
| will be executed *first* or *last* respectively in the hook call loop: |
There was a problem hiding this comment.
What happens when more than one hookimpl are marked as tryfirst or trylast? I believe then LIFO registered order for them applies, but perhaps this should be explicitly mentioned.
There was a problem hiding this comment.
Ok so something like:
Note that
tryfirstandtrylasthooks are still invoked in LIFO order within each category
pluggy/callers.py
Outdated
| If the hook was marked as a ``firstresult`` a single value should | ||
| be set otherwise set a (modified) list of results. Any exceptions | ||
| found during invocation will be deleted. | ||
| """ | ||
| self.result = result |
There was a problem hiding this comment.
Not related to the PR: should we make all attributes private?
There was a problem hiding this comment.
Hmm yeah maybe?
I don't think the original _CallOutcome did this any differently fwiw.
There was a problem hiding this comment.
Oh definitely, this wasn't a criticism of the current code, just something I thought I might bring up.
If we want to have a well-defined API we might want to start doing that somewhat sooner; a lot of problems of the pytest API stem from the fact that public attributes should never been so, but changing them later is a pain because client code now depends on it (even if by accident).
There was a problem hiding this comment.
Yeah totally agree.
I just wasn't sure if .result was initially supposed to be public.
I'm cool to switch it in this PR as long as you guys are.
|
Excellent work btw, my comments are merely to improve this and that, overall everything looks great! Thanks for tackling this. 👍 |
|
oh @nicoddemus should I be flipping the version to |
|
@nicoddemus good enough? |
There was a problem hiding this comment.
oh @nicoddemus should I be flipping the version to .dev already here?
Sure!
| @@ -26,8 +26,12 @@ class HookCallError(Exception): | |||
|
|
|||
| class _Result(object): | |||
| def __init__(self, result, excinfo): | |||
| self.result = result | |||
| self.excinfo = excinfo | |||
| self._result = result | |||
There was a problem hiding this comment.
Just noticed that this broke some tests in pytest, so pytest-dev/pytest#2744 is already paying off. 👍
Resolves a couple docs issues (#49, #64) and adds a new test for
hookimplorder verification.