-
-
Notifications
You must be signed in to change notification settings - Fork 336
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
chore: Sync translations #4136
chore: Sync translations #4136
Conversation
351f248
to
993b608
Compare
It's not clear why, but all the exported translations are now empty. No config was recently changed, so this is odd. |
Somehow all the patch strings were deleted by Crowdin. |
It's really annoying there is no rollback function for the free Crowdin plan. It makes glitches like this a pain. All the translations made in the last 2 days must be manually added again. |
Because the string history is not available with a free plan, it's not clear where it went wrong. But I think it was the regex preprocessor not running or failing for some reason. If anything like this happens again, preprocessing can be moved to the GitHub side so it won't ever upload corrupt or incorrectly modified strings. |
0847eac
to
5ffac9d
Compare
@oSumAtrIX I think the Crowdin action should be split apart into two actions. The first action only syncs to the translation branch (and not create a pull request), but it's scheduled to run every day (or possibly more than once a day). And then a second action would be to create a Crowdin PR. Then, the translation history and changes are available here on GitHub which solves the Crowdin free plan not giving access to history or reverts. |
9b4c82b
to
5505364
Compare
Crowdin can probably be configured to not force push each sync and rather just push the new changes |
Yes that would be helpful. |
I believe you can just change this to the Crowdin branch |
Why would we not want it to create a PR? The PR action in the workflow does not create a new PR if the PR already exists, on first sync the PR will be openened, and on future syncs the PR will remain open to be merged at any time. The "translation history" would be present regardless of the PR. So we only need to schedule a run of the workflow every day. |
The only reason to sync without opening a PR, is when pushing to a branch with a PR then everyone watching this repo will get a daily email on every sync. If the sync is done to a branch with no open PR, then no email spam will be sent. Plus it could run the task every 6 hours or whatever so if Crowdin goes bonkers less translation work is lost since the most recent GitHub artifacts to restore are newer. |
You can disable notifications but also filter for the PR title in the email when receiving a mail from GitHub. Splitting the workflow into two is not elegant. We could make the step that opens a PR only trigger on dispatch or in longer schedules, such as every week. Or completely get rid of the step and open PRs manually. |
Removing the PR part of the pull strings action is ok. But there could be a really simple |
In that case the workflow should include it as a step that is triggered on workflow dispatch. However, at this point, the workflow can also just merge the branch in the first place (A PR would still be good as a standard review process) |
5505364
to
544e4c9
Compare
a PR is definitely still good to have since the AI translations can go off the rails and start adding html and sometimes error messages are interpreted as translations. It's the only time a spot check works and really is needed, since no understanding of the languages is needed to see the problems. |
Some of the spoof strings are outdated, but they'll get updated next time. |
Can we change the crowdin actions now? It was a huge pain restoring Crowdin to it's prior state (and it's still missing some translations). If this happens again the cost needs to be minimized. |
Yes. On the PR step you can add an if guard that checks, if the workflow was triggered by a workflow dispatch event. |
Sync translations from crowdin.com/project/revanced