-
Notifications
You must be signed in to change notification settings - Fork 289
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
Output filtering #808
Output filtering #808
Conversation
Please add proper labels to this PR. Thanks! |
cd586e0
to
e4fbfc9
Compare
@mszostok rebased from master and ready - that was painful! The executor was changed a lot while I was also working on it. |
…to help with analytics reporting.
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.
From the unit-test point of view, the behavior LGTM 👍
However, e2e it doesn't work yet, but as discussed I will go back to testing once you will do the final round of rebasing and rechecking 👍
…r values. - rebase comment.
…hell flags. - rebase comment
…ptured flag value.
85c4ea2
to
a8f78ea
Compare
…input filter runs correctly.
|
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 found one more panic, but besides that it LGTM 👍
Description
Changes proposed in this pull request:
Analytics Reporting
ReportCommand(platform config.CommPlatformIntegration, command string, isButtonClickOrigin, withFilter bool) error
has been extended to includewithFilter
to indicate whether a filter was used when running the command.Testing
Install BotKube:
helm install botkube ./helm/botkube -n botkube --create-namespace /tmp/results-filter.yaml
Run commands on botkube-test channel on Slack:
Related issue(s)
Resolves #742