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

Move translation fetching to gulp action #17827

Merged
merged 3 commits into from
Sep 5, 2023
Merged

Move translation fetching to gulp action #17827

merged 3 commits into from
Sep 5, 2023

Conversation

bramkragten
Copy link
Member

Proposed change

Moved the translation fetching to gulp with the node api from Lokalise.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@github-actions github-actions bot added Dependencies Pull requests that update a dependency file Build Related to building the code labels Sep 5, 2023
@karwosts
Copy link
Contributor

karwosts commented Sep 5, 2023

Tried to add a review comment but github wasn't working... but looks like you have a copy/paste error here:

Lokalise API token is required to download the latest set of Please create a translations.

Copy link
Member

@steverep steverep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 Should we add this to the build chain as well (e.g. if token is defined, fetch from Lokalise, otherwise fetch from nightly)?

build-scripts/gulp/download-translations.js Outdated Show resolved Hide resolved
build-scripts/gulp/download-translations.js Outdated Show resolved Hide resolved
build-scripts/gulp/download-translations.js Show resolved Hide resolved
@bramkragten
Copy link
Member Author

👍🏻 Should we add this to the build chain as well (e.g. if token is defined, fetch from Lokalise, otherwise fetch from nightly)?

Not sure, the nightly translations will be optimised for core, and they take the longest.

@steverep
Copy link
Member

steverep commented Sep 5, 2023

Nightly fails on this branch: https://github.com/home-assistant/frontend/actions/runs/6088732401/job/16520013697#step:6:21

Not sure, the nightly translations will be optimised for core, and they take the longest.

Yeah doesn't really matter I guess. Only advantage is to save a step in the nightly and release actions.

@bramkragten
Copy link
Member Author

bramkragten commented Sep 5, 2023

Copy link
Member

@steverep steverep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6MB off the translations tarball 🎉

@bramkragten bramkragten merged commit 29aed53 into dev Sep 5, 2023
9 checks passed
@bramkragten bramkragten deleted the translations-gulp branch September 5, 2023 22:28
@bramkragten bramkragten removed the Dependencies Pull requests that update a dependency file label Sep 6, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Sep 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Build Related to building the code cla-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants