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

locale:po_to_json: add support for including catalogs from javascript plugins #17725

Conversation

mzazrivec
Copy link
Contributor

When creating gettext catalogs for javascript consumption, we need to merge catalogs from all ManageIQ plugins and convert the result into a json file. So far, we were only able to do that for regular rails engines (i.e. ManageIQ ruby plugins). As things are, we need to include javascript plugins (such as ui-compoments) into the equation as well.

This change makes it possible to download the gettext catalogs from plugins and include them into the merge & conversion process.

@himdel review please?

@miq-bot add_label gaprindashvili/yes

@mzazrivec
Copy link
Contributor Author

I'll create a separate PR for gaprindashvili.

@miq-bot remove_label gaprindashvili/yes
@miq-bot add_label gaprindashvili/no

lang_dir = File.join(plugin_dir, lang)
Dir.mkdir(lang_dir)
lang_file = "#{lang_dir}/#{url.split('/')[-1]}"
system "curl -o #{lang_file} #{url}"
Copy link
Contributor

@himdel himdel Jul 20, 2018

Choose a reason for hiding this comment

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

This should probably use system! (from ManageIQ::Environment)

Right now, all the urls create a file saying: "404: Not Found" and get happily ignored

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. And also added an ensure block at the end of the task (the file deletion part).

@mzazrivec mzazrivec force-pushed the locale_po_to_json_add_support_for_javascript_plugins branch from 6350c2e to 60bac82 Compare July 20, 2018 15:23
@miq-bot
Copy link
Member

miq-bot commented Jul 20, 2018

Checked commit mzazrivec@60bac82 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 8 offenses detected

lib/tasks/locale.rake

@himdel
Copy link
Contributor

himdel commented Jul 20, 2018

Looks like it now handles failures just fine:

himdel@thror:~/manageiq$ be rake locale:po_to_json
** override_gem: manageiq-ui-classic, [{:path=>"/home/himdel/manageiq-ui-classic"}], caller: /home/himdel/manageiq/bundler.d/Gemfile.dev.rb
** Using session_store: ActionDispatch::Session::MemCacheStore
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
curl: (22) The requested URL returned error: 404 Not Found

== Command ["curl -f -o /home/himdel/manageiq/locale/plugins/ui-components/en/ui-components.po https://raw.githubusercontent.com/ManageIQ/ui-components/master/locale/en/ui-components.po"] failed in /home/himdel/manageiq ==

himdel@thror:~/manageiq$ ls locale/
en  en.yml  es  es.yml  fr  fr.yml  ja  ja.yml  manageiq.pot  zanata.xml  zh_CN  zh_CN.yml

👍

@himdel
Copy link
Contributor

himdel commented Jul 20, 2018

Aand now that the files exist (ManageIQ/ui-components#327), confirmed I can see the strings in my manageiq-ui-classic/app/assets/javascripts/locale/en/app.js 👍

@martinpovolny martinpovolny merged commit ce515c6 into ManageIQ:master Jul 20, 2018
@martinpovolny martinpovolny self-assigned this Jul 20, 2018
@martinpovolny martinpovolny added this to the Sprint 91 Ending Jul 30, 2018 milestone Jul 20, 2018
@mzazrivec mzazrivec deleted the locale_po_to_json_add_support_for_javascript_plugins branch July 23, 2018 09:11
@simaishi
Copy link
Contributor

simaishi commented Aug 5, 2018

Backported to Gaprindashvili via #17740

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

Successfully merging this pull request may close these issues.

5 participants