-
Notifications
You must be signed in to change notification settings - Fork 292
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
add support for writing events via the context #1277
Conversation
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.
I have not done a full review of this yet, need to take a break will circle back to this later. Left a couple comments in the context.go in the mean time
Co-authored-by: Louis Ruch <[email protected]>
…ent_test pkg in context_test.go
I pushed a commit (9c7ac25) which refactored a few bits which allowed me to stop exporting a few things. Sorry if it clutters up the PR a bit, but it's all simple stuff. |
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.
Changes look LGTM, just a couple of questions/nits re: tests!
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.
LGTM
This is third of 3 PRs to add the events package and they should be reviewed in order. Before this was #1275 and #1276.
This PR will be merged into the branch for #1276, which will be merged to the branch for #1275, which will be merged intowhen all three have been reviewed/approved.