Skip to content
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

364 adapt text notification #104

Merged
merged 7 commits into from
Jun 20, 2023
Merged

364 adapt text notification #104

merged 7 commits into from
Jun 20, 2023

Conversation

bscheltinga
Copy link
Contributor

@bscheltinga bscheltinga requested a review from nelealbers June 19, 2023 14:20
@bscheltinga
Copy link
Contributor Author

I changed rasa domain files based on suggestions in the testing ticket (PerfectFit-project/testing-tickets#13). Therefore, it is now deviating from the excel files we have in teams (for C1.29, C1.27 and C1.26)

How did we do that so far? Did we also change the excel files on team in that case? I think the content and phrasing should be matching.

Copy link
Contributor

@nelealbers nelealbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I just removed some spaces at the end because I always find those confusing when writing tests.

And yes, I agree that we should update the changes in the excel files on Teams. Maybe not in red (because that suggests that the changes are not incorporated yet) though, but some other color.

Also, good catch that the alternative formulations were not there yet.

Lastly, I don't know if there are any tests with these notifications, but if yes, would be good to update those too.

Co-authored-by: Nele Albers <43779515+nelealbers@users.noreply.github.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@bscheltinga
Copy link
Contributor Author

Thanks @nelealbers, I updated the excel filesl

@bscheltinga bscheltinga merged commit 51c94e7 into main Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notifications adapt text notifications
2 participants