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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Correct Basque language accent typo #4323

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

avayyyyyyy
Copy link

@avayyyyyyy avayyyyyyy commented Jan 17, 2024

馃殌 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:

  • Corrected the typo in the Basque language accent associated with "Mendebalekoa."

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! 馃帀

Fixes common-voice#1761

Replaced incorrect "Araka" with correct "Araba" in Basque language accent associated with "Mendebalekoa."
@avayyyyyyy avayyyyyyy requested a review from a team as a code owner January 17, 2024 17:22
@avayyyyyyy avayyyyyyy requested review from moz-dfeller and removed request for a team January 17, 2024 17:22
@jessicarose
Copy link
Collaborator

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!

Copy link
Collaborator

@jessicarose jessicarose left a 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.

Copy link
Collaborator

@jessicarose jessicarose left a 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?

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] A typo in a Basque accent
2 participants