-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
updated hooks #2850
updated hooks #2850
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2850 +/- ##
=======================================
- Coverage 89% 87% -3%
=======================================
Files 79 79
Lines 7302 7863 +561
=======================================
+ Hits 6514 6811 +297
- Misses 788 1052 +264 |
btw, careful with |
This PR breaks all user callbacks and hooks, is this intended? |
if a user uses positional arguments, then yes :/ |
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. |
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 |
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 |
Ok, I just wanted to make sure we are aware of it. |
No description provided.