-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Improve Slack Sanitization #4475
Conversation
Update 9/9
Latest code merge
@erohmensing not able to get much out of this build error log "Makefile:40: recipe for target 'lint' failed Can you please help me :)? |
@vigneshp826 You have to reformat your files using black (https://github.com/rasahq/rasa#code-style). |
But thank you for your PR, we'll give it a review as soon as possible. |
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.
The blank lines should be there in the docstring, but looks good otherwise.
A changelog entry would be good too please :) |
changelog entry under [Unreleased 1.4.0] - master ? |
@vigneshp826 Yes |
added changelog entry |
rasa/core/channels/slack.py
Outdated
replacement = regex.split("|")[1] | ||
replacement = replacement.replace('>', '') | ||
text = text.replace(regex, replacement) | ||
else: |
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.
The else
part can be dropped.
But it looks good otherwise. Thanks for your contribution! |
Proposed changes:
text generated by slack and substitute it with original content
Status (please check what you already did):
black
(please check Readme for instructions)