frontend icon indicating copy to clipboard operation
frontend copied to clipboard

Fetch nightly translations during build

Open steverep opened this issue 3 years ago • 14 comments

Proposed change

From the discussion in #13381, integrates the nightly translations into the build process:

  • Remove translations from git tracking
  • Add a Gulp task to fetch the translations from the nightly workflow artifacts using the REST API
    • When developer runs any task that builds translations, they will be prompted on the terminal to authenticate on GitHub using the code provided
    • Token will be saved so developer will only have to do this once per local clone (or copy the token around to other locals)
    • Avoids unnecessary API calls and downloads by comparing the creation time of the artifact to a fixed age (currently set to 24 hours but we could probably increase if Lokalise activity is much less)
    • Task runs in about 1-2 sec on my machine when downloading, negligible time otherwise
  • Attempts to integrate this into all workflows
  • Cleans up ESLint rules for build scripts and starts linting them during pre-commit (I got tired of looking at errors and warnings 🍒 )

Type of change

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

Example configuration


Additional information

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

Checklist

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

steverep avatar Sep 13 '22 20:09 steverep

I'm not sure if we should run this automatically each develop build, maybe we should make a separate script command people can run to download the translations

bramkragten avatar Sep 14 '22 08:09 bramkragten

What are your concerns with having it be part of the build? Maybe I can address them.

steverep avatar Sep 14 '22 13:09 steverep

It is just an extra step to get your dev end working and extra work each time, that most people will not need during development I think?

bramkragten avatar Sep 14 '22 14:09 bramkragten

Yeah I get that. I was thinking the same, but come to the opposite conclusion. If the translations are removed from tracking, then everyone must execute the task at least once in order for builds to succeed. So if it's not part of the build, then we rely on new developers following more instructions, and maybe possible issues if my translations != your != the CI's?

I also agree Obviously most developers won't need anything outside English, but they should still have an up to date en.json for the backend.

If you didn't already see too, most times the task will exit without doing anything if you already have recent translations.

steverep avatar Sep 14 '22 14:09 steverep

There's a bug causing inconsistent failures (my favorite kind), so making this a draft until I figure that out.

steverep avatar Sep 14 '22 15:09 steverep

I also agree Obviously most developers won't need anything outside English, but they should still have an up to date en.json for the backend.

Backend translations are not needed right? I think we only have those to support old instances with cast.home-assistant.io? Or do you mean for typing support?

We should just make the translations/en.json from the src/translations/en.json, so english is always up2date. If devs want to work with local translations, they can fetch them with the script.

bramkragten avatar Sep 14 '22 16:09 bramkragten

Okay I fixed the issue with awaiting the extraction so you can test again.

Backend translations are not needed right? I think we only have those to support old instances with cast.home-assistant.io? Or do you mean for typing support?

Now you've exceeded my HA knowledge. I assumed the core translations were necessary to successfully build the frontend. I wasn't even thinking about typing support yet.

We should just make the translations/en.json from the src/translations/en.json, so english is always up2date. If devs want to work with local translations, they can fetch them with the script.

That would be a big departure from the current build script behavior that uses all languages. I thought the goal was just to do the same thing with up to date files, and get rid of tracking them so there's no more accidental edits.

Anyway you could accomplish what you're saying just by setting the SKIP_FETCH_NIGHTLY_TRANSLATIONS environment variable, and forcing the developer to run gulp fetch-nightly-translations beforehand. I don't see the added value there though. If you remove it from the task series altogether, then all the workflows and maybe some other scripts need to be updated.

steverep avatar Sep 15 '22 04:09 steverep

Just for kicks I set the variable to skip and built with an empty translations directory. The build succeeds and you can fire up the UI, but there's lots of missing text or text that doesn't match up with the current version, so clearly the existing translations.js script would need modification for what you are saying.

Or I'm completely misunderstanding and we're on totally different pages 🍎 🍏

steverep avatar Sep 16 '22 03:09 steverep

Just for kicks I set the variable to skip and built with an empty translations directory. The build succeeds and you can fire up the UI, but there's lots of missing text or text that doesn't match up with the current version, so clearly the existing translations.js script would need modification for what you are saying.

Or I'm completely misunderstanding and we're on totally different pages 🍎 🍏

I'll dive in a little deeper this weekend, but if I remember correctly, we always take the src/translations/en.json as base and merge that with the specific lang file, so if that lang file is not there, we should endup with just the base english translations, that is always up to date as it is the source...

bramkragten avatar Sep 16 '22 11:09 bramkragten

Yep, looks like that's what it does, but the build-merged-translations task uses the files in translations/frontend to decide which languages to merge:

gulp.task("build-merged-translations", () =>
  gulp
    .src([inFrontendDir + "/*.json", workDir + "/test.json"], {
      allowEmpty: true,
    })

So if that's empty, there's no output even for English.

steverep avatar Sep 16 '22 14:09 steverep

OK, so we should change that to use the src/translations/translationMetadata.json instead

bramkragten avatar Sep 16 '22 14:09 bramkragten

Looking at the preceding build-master-translation task, it would be easier just to have that one write build/translations/full/en.json, and just skip English in the next task using it as the master. The two tasks are essentially repeating the same work for English.

Alright, so the default clone will be to build English only, and developers can flip an environment flag in devContainer.json or on the terminal to enable multi-language support. Agreed?

Also need to decide:

  1. Builds for cast, demo, and gallery should always fetch the translations since they merge in the backend translations?
  2. Jobs in the CI workflow should fetch the translations as well? Or maybe that's not needed?

steverep avatar Sep 16 '22 17:09 steverep

Please keep in mind, that fetching the backend translations now only works because they are in this repo, by removing them from the repo, like you did in this PR, will make that they will also be removed from the nightly artefacts... They are not fetched from Lokalise

bramkragten avatar Sep 19 '22 08:09 bramkragten

Okay I didn't catch that. I can update that script to pull the core translations as well.

steverep avatar Sep 19 '22 17:09 steverep

Okay here's what I did:

  • New clones will build frontend and supervisor English only (except for those inside GitHub actions)
  • Developer can setup multilingual builds by manually running the "setup & fetch" task to get a token and initial translations once, then future builds will always update them so long as the token is present
  • Builds for demo, cast, and gallery always trigger the fetch since they need the backend translations

Assuming you're good with that, do you think updating with each nightly is okay, or increase 24 to something less often? A good gauge would be how often things change in Lokalise, but I have no idea how to check that.

steverep avatar Oct 12 '22 16:10 steverep

Did you get a chance to examine the latest changes? No rush for beta here - just wanted to know if I should start adding a couple notes to the developer docs.

steverep avatar Oct 26 '22 17:10 steverep

👀 please so I don't have to fix any more conflicts

steverep avatar Nov 29 '22 16:11 steverep