updated hooks

[williamFalcon]

No description provided.

[codecov]

Codecov Report

Merging #2850 into master will decrease coverage by 3%.
The diff coverage is 95%.

@@           Coverage Diff           @@
##           master   #2850    +/-   ##
=======================================
- Coverage      89%     87%    -3%     
=======================================
  Files          79      79            
  Lines        7302    7863   +561     
=======================================
+ Hits         6514    6811   +297     
- Misses        788    1052   +264     

[Borda]

btw, careful with setup and teardown adding extra argument in middle would be breaking changes... :/

[awaelchli]

This PR breaks all user callbacks and hooks, is this intended?
Also it remove the test for a recent fix?
The call to is_implemented is not necessary as it is anyway true always and every hook/callback is a no-op by default, so we can just call it anyway.

[Borda]

This PR breaks all user callbacks and hooks, is this intended?

if a user uses positional arguments, then yes :/

[awaelchli]

no, the user does't call the callback themselves. The user inherits them, so after this change the user has to go to every callback they have implemented and change the signatures manually. This is what you will have to do also in bolts I assume.

[williamFalcon]

yeah, but it’s kind of a necessary evil for 1.0. unless u guys have a way of making it compatible? maybe we can try catch them or something

[williamFalcon]

but i’m pretty sure only the new hooks i introduced have these new args. those hooks aren’t live yet, so it’s a fair change since they only live on master

[awaelchli]

Ok, I just wanted to make sure we are aware of it.