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

phoenix translation sync failing #3603

Closed
phil-davis opened this issue Jun 15, 2020 · 11 comments
Closed

phoenix translation sync failing #3603

phil-davis opened this issue Jun 15, 2020 · 11 comments
Assignees

Comments

@phil-davis
Copy link
Contributor

See issue owncloud/translation-sync#35

Needs someone to investigate.

@phil-davis
Copy link
Contributor Author

@micbar who looks after this sort of thing?

@micbar
Copy link
Contributor

micbar commented Jun 15, 2020

any engineer capable of doing it :-)
@lhirt @kulmann ?

@kulmann
Copy link
Contributor

kulmann commented Jun 15, 2020

It looks like easygettext doesn't support ?? (nullish coalescing operator), which is used in src/store/modal.js.

There are two ways of fixing this:
1: fall back to ||. Need @LukasHirt to clarify, if that is valid or if it breaks his idea of the implementation.
2: add support for parsing ?? upstream, can probably be done similar to this: laget-se/react-gettext-parser@cc9d71d

@LukasHirt
Copy link
Collaborator

LukasHirt commented Jun 15, 2020

1: fall back to ||. Need @LukasHirt to clarify, if that is valid or if it breaks his idea of the implementation.

There shouldn't be a real issue with using OR operator. The idea was to use the nullish coalescing one since in some modal values it is valid to pass false. So it doesn't break anything but simply doesn't look nice when it uses false backup value instead of the one that was passed.

@LukasHirt
Copy link
Collaborator

LukasHirt commented Jun 15, 2020

add support for parsing ?? upstream, can probably be done similar to this: laget-se/react-gettext-parser@cc9d71d

Easygettext seems to be using acorn for parsing the JS code which should support nullish coalescing operator acornjs/acorn#890 Maybe upstream is using some outdated version?

@phil-davis
Copy link
Contributor Author

phil-davis commented Jul 5, 2020

Still failing - e.g. https://drone.owncloud.com/owncloud/translation-sync/1583/33/5

So no new translations are coming.

Nobody has been assigned to this issue.

@LukasHirt
Copy link
Collaborator

Nobody has been assigned to this issue.

I'll take care.

@phil-davis
Copy link
Contributor Author

Translations arrived last night - cce62d1
Works

@phil-davis
Copy link
Contributor Author

Some translation came, but last night: https://drone.owncloud.com/owncloud/translation-sync/1588/33/5

tx INFO: Pushing source file (template.pot)
tx ERROR: Could not upload source file. You can use --skip to ignore this error and continue the execution.
tx ERROR: [Errno 2] No such file or directory: '/drone/src/phoenix/apps/files/l10n/template.pot'
make: *** [l10n-push] Error 1
Makefile:81: recipe for target 'l10n-push' failed

And apps/files/l10n/template.pot does not exist in the phoenix repo.

@LukasHirt or someone ???

@phil-davis phil-davis reopened this Jul 10, 2020
@LukasHirt
Copy link
Collaborator

Agh... I used again the optional chaining operator 🤦 I'll get rid of it. Need to hammer it down my head to stop using them

https://github.com/owncloud/phoenix/blob/41f0791f034a7d94e8076cc02627638fa83a3f1a/apps/files/src/components/LocationPicker/LocationPicker.vue#L215

I'll see if I can get that running upstream with the acorn parser and if yes submit a PR there. It's annoying not to be able to use such nice features.

@LukasHirt
Copy link
Collaborator

Sync is passing again for the past few days - closing

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

No branches or pull requests

4 participants