Add context to Event method, callback handlers and canceling an event #82
Conversation
|
Nice work! I’ll review this after the holidays. |
|
@maxekman Hi! can you review it please? seems to be it's a good piece of functionality |
|
Yes! Sorry, totally forgot about this as I changed jobs this spring. There is some work going on with a generics version, that would be v2 if we merge it. Maybe this change can mark the v1 of the lib? Please rebase this also, thanks. |
|
Looks good on a quick read. Maybe add an example for utilizing the context? |
|
I am looking forward to this feature! I really need this 🚀 |
|
So sorry, I'll get to this this week (and rebase as well). |
|
No stress! Your contribution is much appreciated! |
annismckenzie
force-pushed
the
contextify-fsm
branch
from
f3ead0d
to
38a6e67
Compare
annismckenzie
force-pushed
the
contextify-fsm
branch
from
38a6e67
to
d0046a7
Compare
I tried to do that yesterday but all the examples I came up with are somehow contrived and not useful yet because this is only a preparation MR so I can pull in the rest that's contained in alfaview#1. Would it be okay to merge as is? The rebase is done. |
|
Sure! Let’s do it in that order. |
|
Thanks for picking this up again and contributing! |
|
Let’s do a 1.0 release after your other PR is in. Great push! |
|
Hooray! thanks @annismckenzie @maxekman |
|
The second part of this is up: #88. |
## Description Between v0.3.0 and v1.0.0 (more precisely in looplab/fsm#82), the authors of fsm introduced a new first parameter of type`context.Context` in `Callback`, `Event` and other structures. This PR: - Create a wrapper that discard the first argument and call our callbacks - Add a blank Context ([Background()](https://pkg.go.dev/context#Background)) to all the event we create ## How has this been tested? CI ## Checklist - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
This PR adds a context to the main
Eventmethod. It's also in response to #37 (comment). Finally, it's the first step towards implementing #43 and #38.Adding the context is a breaking change and while the library isn't at v1.0.0 yet it's still an inconvenience. I distinctly didn't add a
CallbacksContextfield because of the following reasons:_ context.ContextOf course, if the maintainer(s)/contributors/the community at large wants this then it's still totally possible.
To see the whole thing in action, potential users can use the full implementation: alfaview#1 (this PR only contains the first part).