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

Download translations dynamically #2120

Merged
merged 1 commit into from Apr 11, 2023

Conversation

jonaharagon
Copy link
Member

@jonaharagon jonaharagon commented Apr 10, 2023

Removes translations from this Git repo, Netlify will download them from Crowdin automatically every time it builds the site (at the expense of making Netlify previews take much longer lol)

@netlify
Copy link

netlify bot commented Apr 10, 2023

Deploy Preview for privacyguides ready!

Name Link
🔨 Latest commit 2df3405
🔍 Latest deploy log https://app.netlify.com/sites/privacyguides/deploys/64359d52a4f046000881a744
😎 Deploy Preview https://deploy-preview-2120--privacyguides.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

jonaharagon pushed a commit that referenced this pull request Apr 10, 2023
@jonaharagon jonaharagon force-pushed the jonaharagon/crowdin-netlify-download branch from 9051363 to 555ac66 Compare April 10, 2023 15:35
@jonaharagon jonaharagon marked this pull request as ready for review April 10, 2023 15:36
@jonaharagon jonaharagon added the l10n Localization label Apr 10, 2023
jonaharagon pushed a commit that referenced this pull request Apr 10, 2023
@jonaharagon jonaharagon force-pushed the jonaharagon/crowdin-netlify-download branch from 555ac66 to aade832 Compare April 10, 2023 15:52
@blacklight447
Copy link
Member

Whats the pro here actually? As i don't think repo size matters that much honestly.

@jonaharagon
Copy link
Member Author

@blacklight447 the advantages are a cleaner commit history and changelog; less manual work since we wouldn't have to check translations on here and Crowdin; less confusion about where files should be edited, because currently there are a lot of files which can not be manually edited on GitHub (only on Crowdin) so there's no reason for them to exist here; changes on Crowdin can be previewed on Netlify much easier (for example changes to the upcoming #2111 Spanish translation would be preview-able live quicker); and that the translated versions of the site will be updated much quicker when changes to the main site are made, instead of outdated information and missing pages sitting on them until the next language release.

The disadvantage is that translations won't be manually reviewed via PR here before going live. I'm unsure whether this is a big risk since we don't accept translations from unapproved contributors (not that the approval process is particularly rigorous, but it is a roadblock to spam bots/trolls at least). Also, longer Netlify build times.

@blacklight447
Copy link
Member

Thanks for clearing it up.

@jonaharagon
Copy link
Member Author

This would also allow us to trigger a new Netlify build as soon as a file hits 100% in Crowdin. https://store.crowdin.com/netlify

@jonaharagon jonaharagon force-pushed the jonaharagon/crowdin-netlify-download branch from aade832 to 2df3405 Compare April 11, 2023 17:47
@jonaharagon jonaharagon merged commit 2df3405 into main Apr 11, 2023
7 of 8 checks passed
@jonaharagon jonaharagon deleted the jonaharagon/crowdin-netlify-download branch April 11, 2023 17:48
@privacyguides-bot privacyguides-bot temporarily deployed to github-pages April 12, 2023 03:46 — with GitHub Actions Inactive
@privacyguides-bot
Copy link
Collaborator

This pull request has been mentioned on Privacy Guides. There might be relevant details there:

https://discuss.privacyguides.net/t/v3-8/12304/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
l10n Localization
Development

Successfully merging this pull request may close these issues.

None yet

3 participants