-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add Crowdin support #154
Add Crowdin support #154
Conversation
538d7f3
to
2c38c19
Compare
Need to specify `languages_mapping` explicitly, because Crowdin doesn't provide a lowercase locale with underscores, as required by Minecraft.
Crowdin will do this automatically, so do it now to avoid unnecessary PRs.
2c38c19
to
ec0b642
Compare
StatusI've tested and iterated on this over on my fork, using a test project on Crowdin. Everything now seems to be working as intended. I've added some user-facing documentation to the README. Note on pre/post processor appsThe processor apps I asked you to install the other day will need to be uninstalled. I encountered a number of issues with them during testing, for example they caused all built translation files to be use the same name ( Frustratingly it seems to take around 12 hours for removing the apps to actually affect how the project builds translations, since they seem to be cached aggressively. Uninstalling them is kinda unintuitive too. You need to go to each on the marketplace, then:
Before mergingYou'll want to setup a few things in the github repo settings:
After mergingNow we'll need to initialize the Crowdin project by uploading what we have in the git repo:
You can manually run a workflow by first clicking on the workflow in the side-bar, then using the "Run workflow" dropdown: Crowdin PRsEvery 12 hours (as configured in The PR will look like this. NotesCrowdin ActionThe available options for the GitHub Action are documented here. They also have some example usage here. PRs vs directly committingIn theory, the download workflow could commit directly to We could consider this if merging PRs becomes a chore. That said I think having that extra review step is not a bad thing. Branch protection
GitHub actions is able to bypass branch protection, allowing it co create/use the Crowdin branchesWe all know about git branches, but it's worth highlighting that Crowdin also has its own "branch" concept (configured via This could prove useful should we need to maintain different translations for different freecam/minecraft versions. Currently, the CI jobs are configured to only run on |
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! This should hopefully make things easier for our translators.
This PR is somewhat untested.The GitHub Actions Workflows in particular are based on these examples and this video,but untested. I'll try and get round to testing them on my fork & using my test Crowdin project, but CI is always a pain to test. EDIT: done.This will do nothing without setting up a Crowdin project, enabling "Allow GitHub Actions to create and approve pull requests", creating & adding a Crowdin personal access token with access to the Crowdin project. This is described in the gist I mentioned on discord.
One potential pitfall is allowing two-way translation sync. This could potentially result in repo changes clobbering Crowdin changes, if something is changed on crowdin, but then we also change something else in the repo and upload the result.
Best practice is probably to upload only the source (
en_us.json
), so that the translations are only ever modified by Crowdin. I thought it might be nice to allow translators to use GitHub if they prefer it to Crowdin, however this may be opening up a can of worms...This can be avoided with
upload_translations: false
on all workflows.Fixes #145