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

Fix script/translations_develop launch + clean some unused error i18n #25459

Merged
merged 3 commits into from
Aug 12, 2019

Conversation

Quentame
Copy link
Member

@Quentame Quentame commented Jul 24, 2019

Breaking Change:

Nothing

Description:

Fixing translation files when going to develop and launch script/translations_develop :

  • strings.json property missing
  • [i18n].json property missing
  • [i18n].json unused property
  • property order

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

@ghost
Copy link

ghost commented Jul 24, 2019

Hey there @andrewsayre, mind taking a look at this pull request as its been labeled with a integration (heos) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@ghost
Copy link

ghost commented Jul 24, 2019

Hey there @robbiet480, mind taking a look at this pull request as its been labeled with a integration (upnp) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@ghost
Copy link

ghost commented Jul 24, 2019

Hey there @elupus, mind taking a look at this pull request as its been labeled with a integration (arcam_fmj) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@ghost
Copy link

ghost commented Jul 24, 2019

Hey there @fredrike, mind taking a look at this pull request as its been labeled with a integration (tellduslive) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@ghost ghost assigned Adminiuga and dmulcahey Jul 24, 2019
@ghost
Copy link

ghost commented Jul 24, 2019

Hey there @frenck, mind taking a look at this pull request as its been labeled with a integration (twentemilieu) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@ghost
Copy link

ghost commented Jul 24, 2019

Hey there @dmulcahey, @Adminiuga, mind taking a look at this pull request as its been labeled with a integration (zha) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@balloob
Copy link
Member

balloob commented Jul 24, 2019

Our files are managed by running script/download, so all changes to files in .translations will be lost.

Why would we change the property order in strings.json?

@Quentame
Copy link
Member Author

Hi @balloob

I've change order on heos, twentemilieu, UPNP, Vesync and ZHA strings.json because I though it should be on the same as generated but I'm wrong, reverting this.

The purpose of this PR is tho fix this :
Capture d’écran 2019-07-25 à 12 45 05

while launching script/translations_develop on clean dev, that's all.

@andrewsayre andrewsayre removed their request for review July 25, 2019 16:25
"step": {},
"error": {},
"abort": {}
"title": "Arcam FMJ"
Copy link
Member

Choose a reason for hiding this comment

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

we should remove this whole file, there is no config flow.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Oh I guess it's correct. It just only works with import and not with user facing config flow. my bad.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, it's right ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we resolve this @balloob ?

@@ -1,6 +1,7 @@
{
"config": {
"abort": {
"all_configured": "TelldusLive is already configured",
Copy link
Contributor

Choose a reason for hiding this comment

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

Have the config-flow for tellduslive been updated with this?

Copy link
Member Author

Choose a reason for hiding this comment

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

In all tellduslive/.translations/[LANG].json, this property is contained.

I've picked this one from the en.json file.

So I guess yes, but didn't look at it.

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK Ait is not used in the flow so this key should not be there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okey, so I remove the key for all langs

Copy link
Member Author

Choose a reason for hiding this comment

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

I've fixed your comment

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we resolve this @fredrike ?

@Quentame
Copy link
Member Author

First, there is #25575 ...

@MartinHjelmare MartinHjelmare changed the title Fix : script/translations_develop launch + clean some unused error i18n Fix script/translations_develop launch + clean some unused error i18n Aug 9, 2019
@MartinHjelmare
Copy link
Member

Please rebase and solve the merge conflicts.

@Quentame
Copy link
Member Author

Quentame commented Aug 9, 2019

Done @MartinHjelmare

@MartinHjelmare
Copy link
Member

Ok @balloob ?

@balloob
Copy link
Member

balloob commented Aug 10, 2019

When I run translations_download, commit the changes and then run translations_develop, it looks like you missed one:

home-assistant/ on pr/25459
› script/translations_download
Requesting...��
Remote https://s3-eu-west-1.amazonaws.com/lokalise-assets/files/export/130246255a974bd3b5e8a1.51616605/907110db293cda55ea04ae3db81deb03/Home_Assistant_-_Backend-locale.zip... OK
Local Home_Assistant_-_Backend-locale.zip... OK
Unzipped /opt/dest/locale, /opt/dest/locale/ru.json, /opt/dest/locale/zh-Hant.json, /opt/dest/locale/zh-Hans.json, /opt/dest/locale/en.json, /opt/dest/locale/ja.json, /opt/dest/locale/es-419.json, /opt/dest/locale/de.json, /opt/dest/locale/fr.json, /opt/dest/locale/pt-BR.json, /opt/dest/locale/ar.json, /opt/dest/locale/ko.json, /opt/dest/locale/it.json, /opt/dest/locale/nl.json, /opt/dest/locale/tr.json, /opt/dest/locale/fa.json, /opt/dest/locale/pl.json, /opt/dest/locale/th.json, /opt/dest/locale/sv.json, /opt/dest/locale/id.json, /opt/dest/locale/nn.json, /opt/dest/locale/no.json, /opt/dest/locale/ca.json, /opt/dest/locale/cs.json, /opt/dest/locale/he.json, /opt/dest/locale/da.json, /opt/dest/locale/fi.json, /opt/dest/locale/uk.json, /opt/dest/locale/ro.json, /opt/dest/locale/hu.json, /opt/dest/locale/vi.json, /opt/dest/locale/bg.json, /opt/dest/locale/af.json, /opt/dest/locale/lt.json, /opt/dest/locale/lv.json, /opt/dest/locale/et.json, /opt/dest/locale/is.json, /opt/dest/locale/lb.json, /opt/dest/locale/cy.json, /opt/dest/locale/eu.json, /opt/dest/locale/sl.json, /opt/dest/locale/es.json, /opt/dest/locale/pt.json OK

home-assistant/ on pr/25459
› git status
On branch pr/25459
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   homeassistant/components/adguard/.translations/fr.json
	modified:   homeassistant/components/deconz/.translations/zh-Hant.json
	modified:   homeassistant/components/homekit_controller/.translations/no.json
	modified:   homeassistant/components/hue/.translations/cy.json
	modified:   homeassistant/components/hue/.translations/fr.json
	modified:   homeassistant/components/life360/.translations/fr.json
	modified:   homeassistant/components/met/.translations/fr.json
	modified:   homeassistant/components/ps4/.translations/no.json
	modified:   homeassistant/components/somfy/.translations/fr.json
	modified:   homeassistant/components/tellduslive/.translations/ca.json
	modified:   homeassistant/components/tellduslive/.translations/da.json
	modified:   homeassistant/components/tellduslive/.translations/de.json
	modified:   homeassistant/components/tellduslive/.translations/en.json
	modified:   homeassistant/components/tellduslive/.translations/es-419.json
	modified:   homeassistant/components/tellduslive/.translations/es.json
	modified:   homeassistant/components/tellduslive/.translations/fr.json
	modified:   homeassistant/components/tellduslive/.translations/hu.json
	modified:   homeassistant/components/tellduslive/.translations/it.json
	modified:   homeassistant/components/tellduslive/.translations/ko.json
	modified:   homeassistant/components/tellduslive/.translations/lb.json
	modified:   homeassistant/components/tellduslive/.translations/nl.json
	modified:   homeassistant/components/tellduslive/.translations/no.json
	modified:   homeassistant/components/tellduslive/.translations/pl.json
	modified:   homeassistant/components/tellduslive/.translations/pt.json
	modified:   homeassistant/components/tellduslive/.translations/ru.json
	modified:   homeassistant/components/tellduslive/.translations/sl.json
	modified:   homeassistant/components/tellduslive/.translations/sv.json
	modified:   homeassistant/components/tellduslive/.translations/zh-Hans.json
	modified:   homeassistant/components/tellduslive/.translations/zh-Hant.json
	modified:   homeassistant/components/tradfri/.translations/fr.json
	modified:   homeassistant/components/tradfri/.translations/no.json
	modified:   homeassistant/components/upnp/.translations/da.json
	modified:   homeassistant/components/upnp/.translations/de.json
	modified:   homeassistant/components/upnp/.translations/es.json
	modified:   homeassistant/components/upnp/.translations/hu.json
	modified:   homeassistant/components/upnp/.translations/lb.json
	modified:   homeassistant/components/upnp/.translations/nl.json
	modified:   homeassistant/components/upnp/.translations/nn.json
	modified:   homeassistant/components/upnp/.translations/no.json
	modified:   homeassistant/components/upnp/.translations/pl.json
	modified:   homeassistant/components/upnp/.translations/pt.json
	modified:   homeassistant/components/upnp/.translations/ro.json
	modified:   homeassistant/components/upnp/.translations/sl.json
	modified:   homeassistant/components/upnp/.translations/sv.json

Untracked files:
  (use "git add <file>..." to include in what will be committed)

	homeassistant/components/notion/.translations/cy.json
	homeassistant/components/onboarding/.translations/vi.json
	homeassistant/components/plaato/.translations/fr.json
	homeassistant/components/wemo/.translations/fr.json
	homeassistant/components/wwlln/.translations/cy.json

no changes added to commit (use "git add" and/or "git commit -a")

home-assistant/ on pr/25459
› git add -A

home-assistant/ on pr/25459
› git commit -m "WIP"
black................................................(no files to check)Skipped
[pr/25459 67ec38365] WIP
 49 files changed, 165 insertions(+), 26 deletions(-)
 create mode 100644 homeassistant/components/notion/.translations/cy.json
 create mode 100644 homeassistant/components/onboarding/.translations/vi.json
 create mode 100644 homeassistant/components/plaato/.translations/fr.json
 rewrite homeassistant/components/somfy/.translations/fr.json (82%)
 create mode 100644 homeassistant/components/wemo/.translations/fr.json
 create mode 100644 homeassistant/components/wwlln/.translations/cy.json

home-assistant/ on pr/25459
› script/translations_develop

home-assistant/ on pr/25459
› git status
## pr/25459
 M homeassistant/components/tellduslive/.translations/en.json

home-assistant/ on pr/25459
› git diff
diff --git a/homeassistant/components/tellduslive/.translations/en.json b/homeassistant/components/tellduslive/.translations/en.json
index c2b005618..4ed9ef597 100644
--- a/homeassistant/components/tellduslive/.translations/en.json
+++ b/homeassistant/components/tellduslive/.translations/en.json
@@ -1,7 +1,6 @@
 {
     "config": {
         "abort": {
-            "all_configured": "TelldusLive is already configured",
             "already_setup": "TelldusLive is already configured",
             "authorize_url_fail": "Unknown error generating an authorize url.",
             "authorize_url_timeout": "Timeout generating authorize url.",

@Quentame
Copy link
Member Author

Will look at it today or tomorrow

@Quentame
Copy link
Member Author

Capture d’écran 2019-08-11 à 17 47 51

@balloob : I don't see what's wrong, are you on the last version of my branch ?

@balloob
Copy link
Member

balloob commented Aug 12, 2019

I think it was because the key still existed in Lokalise. I have now removed it from there and will go ahead and merge this. Great work 👍

@balloob balloob merged commit ec2ce31 into home-assistant:dev Aug 12, 2019
@lock lock bot locked and limited conversation to collaborators Aug 13, 2019
@Quentame Quentame deleted the fix-translations branch August 17, 2019 11:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.