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

Add Crowdin support #154

Merged
merged 4 commits into from
Jan 21, 2024
Merged

Conversation

MattSturgeon
Copy link
Member

@MattSturgeon MattSturgeon commented Jan 9, 2024

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

@MattSturgeon MattSturgeon added enhancement New feature or request i18n Internationalization: languages & translation labels Jan 9, 2024
@MattSturgeon MattSturgeon force-pushed the i18n/init branch 3 times, most recently from 538d7f3 to 2c38c19 Compare January 18, 2024 09:05
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.
@MattSturgeon
Copy link
Member Author

MattSturgeon commented Jan 18, 2024

Status

I'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 apps

The 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 (zh-CN.json) and also ignore the languages mapping we configure.

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:

  • Click "Install"
  • Select your account
  • Click "Uninstall"
  • Select "Freecam"

Before merging

You'll want to setup a few things in the github repo settings:

After merging

Now we'll need to initialize the Crowdin project by uploading what we have in the git repo:

  • Go to the repo's Actions page.
  • Manually run the "Crowdin (Upload Sources)" workflow
  • Manually run the "Crowdin (Upload Translations)" workflow

You can manually run a workflow by first clicking on the workflow in the side-bar, then using the "Run workflow" dropdown:

Crowdin PRs

Every 12 hours (as configured in crowdin-download-translations.yaml), the download workflow will build & download translations from Crowdin, then it'll commit any changes to the i18n/main branch and update/open a PR if there are any differences with main.

The PR will look like this.

Notes

Crowdin Action

The available options for the GitHub Action are documented here. They also have some example usage here.

PRs vs directly committing

In theory, the download workflow could commit directly to main, instead of i18n/main, avoiding the need for a PR.

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

I'm not sure if the download workflow will be allowed to push to i18n/main because of the ** branch protection.

GitHub actions is able to bypass branch protection, allowing it co create/use the i18n/main branch. Actions kinda have admin perms.

Crowdin branches

We all know about git branches, but it's worth highlighting that Crowdin also has its own "branch" concept (configured via crowdin_branch_name). This is essentially a top-level directory at the root of the crowdin project, allowing us to keep seperate "branches" of our translations.

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 main, but other branches such as 1.19 could also be listed in future.

Copy link
Collaborator

@hashalite hashalite 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! This should hopefully make things easier for our translators.

@hashalite hashalite merged commit dab771c into MinecraftFreecam:main Jan 21, 2024
@MattSturgeon MattSturgeon deleted the i18n/init branch April 29, 2024 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request i18n Internationalization: languages & translation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crowdin localization
2 participants