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

es: CR2 Day 1 translations #1857

Merged
merged 6 commits into from
Mar 1, 2024
Merged

es: CR2 Day 1 translations #1857

merged 6 commits into from
Mar 1, 2024

Conversation

henrif75
Copy link
Collaborator

@henrif75 henrif75 commented Feb 28, 2024

Professional translations for CR2 content.
Changes are being staged in a separate branch until all Spanish content is reviewed.

#1463 #284

@henrif75 henrif75 marked this pull request as ready for review February 29, 2024 00:04
@deavid
Copy link
Collaborator

deavid commented Feb 29, 2024

Both PRs #1857 and #1858 suffer from the same problems: The review step is showing an error for unintended msgid changes, and 13000 lines removed plus over 1000 lines added.

Even via console I don't know really how to review this. It doesn't look like we're changing the translations, instead it seems we're directly replacing the whole translation files.

I would kindly ask if this can be made in smaller diffs that are more comprehensive. If not, it will take me several hours to do a review as I'd probably will need to build the whole course and compare manually to the website that it builds instead of the code.

Also a quick search of a random old translation shows that it doesn't exist on the new one. I just searched for "Operator Overloading" and seems that it is lost.

@henrif75
Copy link
Collaborator Author

Both PRs #1857 and #1858 suffer from the same problems: The review step is showing an error for unintended msgid changes, and 13000 lines removed plus over 1000 lines added.

Even via console I don't know really how to review this. It doesn't look like we're changing the translations, instead it seems we're directly replacing the whole translation files.

I would kindly ask if this can be made in smaller diffs that are more comprehensive. If not, it will take me several hours to do a review as I'd probably will need to build the whole course and compare manually to the website that it builds instead of the code.

Also a quick search of a random old translation shows that it doesn't exist on the new one. I just searched for "Operator Overloading" and seems that it is lost.

Hi @deavid, thanks for your feedback.
I redid the diff and kept the obsolete entries (#~) so the diff can be less daunting. We can remove them later when merging back to main.
It also eliminated the unintended check error.

po/es.po Outdated Show resolved Hide resolved
po/es.po Outdated Show resolved Hide resolved
po/es.po Outdated Show resolved Hide resolved
po/es.po Outdated Show resolved Hide resolved
po/es.po Outdated Show resolved Hide resolved
po/es.po Outdated Show resolved Hide resolved
po/es.po Outdated Show resolved Hide resolved
po/es.po Outdated Show resolved Hide resolved
po/es.po Outdated Show resolved Hide resolved
po/es.po Outdated Show resolved Hide resolved
@deavid
Copy link
Collaborator

deavid commented Mar 1, 2024

All reviews done. Thanks for making the diffs easier to manage.

@henrif75 henrif75 requested a review from deavid March 1, 2024 18:47
@henrif75
Copy link
Collaborator Author

henrif75 commented Mar 1, 2024

@deavid Thank you!

@henrif75 henrif75 merged commit 65e1e1d into google:es-cr2 Mar 1, 2024
33 checks passed
@henrif75 henrif75 deleted the es-cr2-day1 branch March 1, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants