-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
feat: bake translations as part of the build processes #28483
Conversation
Tweaked the title. :) I fear that this won't cover all use cases (e.g. people running from the repo). I suppose this could be done in package.json as part of |
5a75403
to
6ac1fc5
Compare
Oh I like the idea of bringing the frontend part in there, looks like there's already other translation-related stuff there we can centralize/simplify |
e2c1df8
to
e90e52d
Compare
@@ -1128,6 +1128,9 @@ def test_get_column_names_from_metric(self): | |||
"my_col" | |||
] | |||
|
|||
@pytest.mark.skip( |
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.
@dpgaspar I think you may have written those 2 tests before that I'm disabling here. I believe these tests were looking for a failure of some kind, but were (are at least currently were) picking up on the language pack entry for "Error message". Turns out the "bootstrap payload" is including the language pack for the current locale, and if it happens to be en
it included a lot of empty strings with empty mappings. I'm now making sure that the en
language pack has an empty payload as opposed to a map with empty targets.
Now i'm thinking these 2 unit tests are wrong, and maybe unneeded, so I'm skipping them here and sending this message in a bottle to figure out if we want to follow up with anything on those.
@michael-s-molina @villebro , looking to merge this - let me know if you see any breaking change. Note that I added an entry in |
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.
Seems like we're having a hard time getting more eyes on this, but from what I see, this seems like a good improvement. If there is anything that would be considered a breaking change here, it seems like it'll be patchable to address those issues before 5.0.
8985879
to
c8b3f4b
Compare
I think it comes down to - if you build from repo, you have to run an extra [documented] command now. Oh also for apache releases I added a command to run, so as long as the release manager follows instructions we're good. |
Just wanted to come in and say I love this, thank you so much for all the effort! It's much appreciated and I look forward to the next release. It's going to make our build process so much easier. |
There's been some confusion around the state of the translations in the repo as well as within different builds (docker, pypi, official releases,...).
In this PR, we assume that "compiled" or redundant files that can be generated don't belong in the repository, and should be baked as part of the build process.
Done here:
.mo
(binary files compile from .po files, and used by the backend / pybable), .json files (used by the frontend/jed) aren't in the repo anymorenpm run build-translation
script so it's bundled with the app in superset-frontenden
that's empty, as opposed to a large json file of about 200kb that has a map of empty translation, this has a positive impact on the bootstrap payload size in original page load for theen
locale