Detailed error message on hook registration conflict#379
Conversation
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
Thank you for getting this started, the pr is well prepared.
Please have a look at my comments.
| ) -> None: | ||
| assert not self.has_spec() | ||
| if self.spec is not None: | ||
| raise RuntimeError( |
There was a problem hiding this comment.
This ough to be a ValueError or a subclass of it
| pass | ||
|
|
||
| pm.add_hookspecs(Api1) | ||
| with pytest.raises(RuntimeError): |
There was a problem hiding this comment.
Please add a check for the message, just to be safe
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
Well done, would you like to compact history or squash?
|
I usually do squash on merge. Should I do it myself? |
|
We currently don't have squash merge enabled on the repo, but of you consent to squash I'll happily do it later today after enabling |
nicoddemus
left a comment
There was a problem hiding this comment.
LGTM, would you like to add a CHANGELOG entry as well?
No thank you, it's minor fix. Still can add line if it's required. |
Hi! I've spent some time trying to nail down hook registration error:
This PR raises
RuntimeErrorwith detailed message which should ease developer's life a bit: