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

Updates deprecated Old Tupi (tpw) to Tupinambá (tpn) #3158

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

DJ-Saidez
Copy link
Member

@DJ-Saidez DJ-Saidez commented Jan 26, 2025

This issue seeks to solve #3140
This was almost entirely based off jiru's fix #2900

$audioBasePath = Configure::read('Recordings.path');
if (is_dir($audioBasePath.DS.$from)) {
rename($audioBasePath.DS.$from, $audioBasePath.DS.$to);
}
Copy link
Member

@jiru jiru Feb 1, 2025

Choose a reason for hiding this comment

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

There is no need to rename audio directories anymore since #2880 was merged. This is because audio files are no longer placed into lang subdirectories.


class OldTupiCodeRename extends AbstractMigration
{
private $langColumns = [
Copy link
Member

Choose a reason for hiding this comment

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

In theory you want to add the new column

'audios' => ['sentence_lang'],

let’s say, for the sake of completeness. But in practice it’s not really necessary because we don’t have any audio in Old Tupi.

@DJ-Saidez
Copy link
Member Author

DJ-Saidez commented Feb 1, 2025

Would you like me to make those changes, or are you just mentioning those for posterity?

Also, where exactly would that new column be added if there was existing audio?

@jiru
Copy link
Member

jiru commented Feb 1, 2025

Well it’s your pull request so you have to make the suggested changes (unless you ticked "allow maintainters to modify this PR" setting when you created it). My initial thought was that you take care of the flags and language name changes and I handle the code part, but since you added a migration script to your pull request, I take it that you want to contribute some code too, which is great by the way, so I am reviewing it.

The "column" I mentioned is a column in the SQL table that contains data about audio recordings.
The audios table goes like this in SQL terms:

CREATE TABLE `audios` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `sentence_id` int(11) NOT NULL,
  `sentence_lang` varchar(4) DEFAULT NULL,
  `user_id` int(11) DEFAULT NULL,
  `external` varchar(500) DEFAULT NULL,
  `created` datetime NOT NULL,
  `modified` datetime NOT NULL,
  PRIMARY KEY (`id`),
  KEY `sentence_id` (`sentence_id`)
);

So there are 7 columns, one of them sentence_lang contains the language of the sentence associated to that audio. The migration script takes care of going through all the records of all the tables in Tatoeba’s database and renames language codes wherever they are. The script has to know what columns in what tables contain language codes. We tell it by setting the $langColumns attribute to the various locations and 'audios' => ['sentence_lang'], is an additional one, it basically means "column sentence_lang in the table audios".

@DJ-Saidez
Copy link
Member Author

DJ-Saidez commented Feb 3, 2025

I take it that you want to contribute some code too

My programming capabilities are basic at best, I took one semester of Python, so maybe not much that I would be able to do 😅
I do appreciate the reviews and comments though, this makes it very helpful both for myself and any other future contributors!

'contributions' => ['sentence_lang', 'translation_lang'],
'contributions_stats' => ['lang'],
'languages' => ['code'],
'audios' => = ['sentence_lang'],
Copy link
Member

Choose a reason for hiding this comment

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

There is a typo here. It should be

'audios' => ['sentence_lang'],

without the extra equal sign.

@jiru
Copy link
Member

jiru commented Feb 3, 2025

And one last thing, the migration file should be named with a .php extension, e.g. 20250125_OldTupiCodeRename.php.

@jiru
Copy link
Member

jiru commented Feb 3, 2025

There is also a problem with the name of your migration script file. The prefix number 20250125 is used to compare against other migration scripts in config/Migrations/ and see which one is older or newer in order to apply migrations in correct order. Since 20250125 is a lower number than all the other 14-digit numbers (such as 20190401052133), it is considered to be the oldest, and that confuses the program that applies migrations. Normally the creation of migration files is automated with the help of [cake bake](https://book.cakephp.org/migrations/3/en/index.html#creating-migrations) so you don’t have to care about such details.

@jiru jiru added this to the next milestone Feb 4, 2025
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.

2 participants