-
Notifications
You must be signed in to change notification settings - Fork 765
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
Emit events before and after executing a message #577
Conversation
I just noticed we lack a comprehensive doc for the events. The API docs don't currently include the events. Should we maybe collect a list of the events in the README? Not sure where would be a good place for that.
Hm, the easiest would probably be in the but on the other hand, those tests are rather a black-box and we don't know the expected result for each message in advance |
Yes, events should really be documented (think we had some basic version before we switched to auto-generated docs, not sure?). Some are some techniques to do this with documentation, but maybe for now some manual addition to README will also do as @s1na suggested? For tests I would find this more fitting as a new file in Update: did a quick search and found e.g. this on former event documentation, maybe we should simply do an adopted re-integration of the text in the README? |
Hi @alcuadrado, will you be able to add some docs and tests on this soon, so that we eventually can include in a subsequent |
c2707bf
to
82bc939
Compare
I added tests for all of the events in this PR, and added documentation about them in the README. |
82bc939
to
bbcd24c
Compare
Oops, this was outdated wrt master. I rebased it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks! 😄
@s1na @alcuadrado I would now prepare release notes for a |
Sounds good! Thanks |
This is a WIP PR, as I'm not sure about
tests/api
?Closes #574