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

Correct second person message #4228

Merged
merged 4 commits into from
Nov 14, 2023
Merged

Conversation

stuartlangridge
Copy link
Contributor

To fix #4115

Pull Request Form

Type of Pull Request

  • Bulk sentence upload
  • Related to a listed issue
  • Other

Acknowledging contributors

  • Jess Rose

@moz-rotimib
Copy link
Contributor

Hello @stuartlangridge thank you for your contribution.

Could you update this test to look for You want to join the Common Voice mailing list instead? You just need to update I to You in the test assertion and this should be good to merge.

Once again thanks for your contribution and let me know if you have any questions

Corresponding with change to web/locales/en/messages.ftl `confirm-join-mailing-list`
@stuartlangridge stuartlangridge requested a review from a team as a code owner November 13, 2023 23:29
@stuartlangridge stuartlangridge requested review from moz-rotimib and removed request for a team November 13, 2023 23:29
@stuartlangridge
Copy link
Contributor Author

Could you update this test to look for You want to join the Common Voice mailing list instead? You just need to update I to You in the test assertion and this should be good to merge.

PR updated as requested!

@moz-rotimib
Copy link
Contributor

moz-rotimib commented Nov 14, 2023

Hello @stuartlangridge it seems you also need to make the same change you just made here and here

Sorry I didn't see these earlier. The tests should be fine after these changes are made.

Corresponding with change to web/locales/en/messages.ftl `confirm-join-mailing-list`; there are two tests in this file, it seems
And a third use of the same text
@stuartlangridge
Copy link
Contributor Author

Hello @stuartlangridge it seems you also need to make the same change you just made here and here

Sorry I didn't see these earlier. The tests should be fine after these changes are made.

Yup, my fault for not checking :) I think they're all changed now.

Copy link
Contributor

@moz-rotimib moz-rotimib left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks for your contribution 🎉

@moz-rotimib moz-rotimib merged commit 7cf2b00 into common-voice:main Nov 14, 2023
2 checks passed
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.

[BUG] Pronoun disagreement on dataset download page
2 participants