-
Notifications
You must be signed in to change notification settings - Fork 885
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
Extends Transition words list for Turkish after user feedback #21616
base: trunk
Are you sure you want to change the base?
Extends Transition words list for Turkish after user feedback #21616
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 don't know why a lot of the unit tests were commented out in this file, but I wanted to uncomment the Turkish ones, and then decided to do it for all languages.
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 no idea as well, but agree! I think these language-specific tests are useful 🙌🏽
…ck-suggested-turkish-transition-words
Pull Request Test Coverage Report for Build 310b9142a36ed2dfbe568c02eb4beb55db4e0c8eDetails
💛 - Coveralls |
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.
Thank you for the very thorough investigation of the suggested Turkish transition words! And, it's also great to see your proactiveness in making related changes in English too! 🙌🏽
I agree with your observation and reasonings in the document. I left one comment about "if" on its own whether it's a transition word.
About the changelog items, seeing that the changes for English have an impact now on the transition word detection in English, I think that a general changelog item (for the Free plugin) should also be added. The changes are user-facing :)
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 no idea as well, but agree! I think these language-specific tests are useful 🙌🏽
Do I understand correctly that you mean that the changelog should mention that it applies not only to Turkish, but also to English? If so, I agree! I didn't add it because there are only a few changes to the English list, but a change is a change :) Edit: I updated the changelog entries ✅ |
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
according to
.Geçiş kelimeleri
=Transition words
sayesinde
,madem
aksi durumda
,öte yandan
ne ne
,olsun olmasın
Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
UI changes
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes #21532