-
Notifications
You must be signed in to change notification settings - Fork 816
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Correct Basque language accent typo #4323
base: main
Are you sure you want to change the base?
Conversation
Fixes common-voice#1761 Replaced incorrect "Araka" with correct "Araba" in Basque language accent associated with "Mendebalekoa."
Thanks so much for your quick work! So this change impacts a previous migration and would only deliver this updated accent description for people running brand new setups of the MCV project. You'll want to write a new migration that creates a new .ts file that will sit in the same directory alongside the existing, unchanged 20211110104012-accent-tables-in-db.ts file and the other past migration files to fix this. I don't have much experience with this process myself, yet. But will be having a look at this process later today (to better update documentation around it) and should be able to offer better support around this fix later in the week, if needed! |
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.
This change was made in the migration file from 2021 that first introduced the typo. This fix would only work for new setups of the Common Voice project.
I would recommend reverting this file (20211110104012-accent-tables-in-db.ts) to its original state and writing a new migration that would create a new .ts file that should sit in the /model/db/migrations/ folder alongside the other migration files.
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 so much and my apologies to have kept you waiting during some staff travel.
Could I request two more smallish changes?
Firstly, the filename contains the timestamp "20220124120000" which is going to show up as 2022, could you change the filename to match the current year and date? Ex 20240206120000 if we wanted it to correspond to today's date.
Also, while you've done a beautiful job with this, could I request a small change in line with best practices we're trying to enforce throughout the project?
A typo can be pretty risky with changes like these, so using the accent token instead of the long string accent name to refer to location of changes for lines 8 and 18 would reduce a lot of risk, would it be possible to have you change this to "AND accent_token = 'mendebalekoa'" instead of the longer accent_name string?
馃殌 Pull Request Summary:
Fixes #4287
Hey team! 馃憢
I've just addressed a long-standing issue reported in #4287 . There was a small typo in the Basque language accent description, where "Araka" was mistakenly used instead of the correct term "Araba."
Changes Made:
I've tested the fix locally, and everything looks good.
馃攳 Reviewers, your input is appreciated!
Please take a look, and if everything checks out, feel free to merge it in.
Thanks for assigning this issue to me! Excited to dive in and contribute. If there are more issues in a similar vein, I'm eager to take them on!
Cheers! 馃帀