[GitHub] alanmcruickshank commented on a change in pull request #3750: Consolidate all translation config

[GitBox <gi...@apache.org>]

alanmcruickshank commented on a change in pull request #3750: Consolidate all translation config
URL: https://github.com/apache/incubator-superset/pull/3750#discussion_r148240442
 
 

 ##########
 File path: CONTRIBUTING.md
 ##########
 @@ -375,6 +376,11 @@ Execute this command to convert the en PO file into a json file:
 
     po2json -d superset -f jed1.x superset/translations/en/LC_MESSAGES/messages.po superset/translations/en/LC_MESSAGES/messages.json
 
+If you get errors running `po2json`, you might be running the ubuntu package with the same
+name rather than the nodejs package (they have a different format for the arguments). You
+need to be running the nodejs version, and so if there is a conflict you may need to point
+directly at `/usr/local/bin/po2json` rather than just `po2json`.
 
 Review comment:
   I did install using using `npm install -g`, as that's what it currently recommends in `CONTRIBUTING.md`. Should we update that to not recommend a global install?
   
   I wonder if actaully I should update the docs here to add clarity around what needs to be done to makes sure `po2json` is installed in the right place more broadly. I found the process quite fiddly for someone who doesn't use node very often - global vs local installs and what folder things are available in was at the root of that I think. I'd love to make the process as easy as:
   
   1. Run command X in folder Y to install Z.
   2. Run command A in folder B to convert messages.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services