-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Is google sheet referenced in Makefile still required for this project? #1
Comments
Thanks for this! Yeah, the Google sheet isn't really documented and was a shortcut for being able to load translations quickly from a sheet multiple translators were editing. It's also using a Python script which might be simpler to remove or rewrite in node. Moving to npm probably makes sense, yarn is mostly force of habit for me and I think is in the Gatsby starter. The benefit of the Makefile for the deploy is that it makes multiline scripts easier, in particular the setup for cache invalidation by language here. It's also something I'm used to though, so it may be simpler to do in a node script |
Thanks for the explanation! IMO re: rewriting stuff from python to node or from Makefile into the package.json I think just documenting the purpose of these pieces is fine, I don’t see a need to rewrite them at this time. But I do think being consistent with npm and/or yarn would be helpful. I can also add stuff to the docs about installing aws CLI, since thats a dependency for deploying that currently isn’t documented. Thanks again for filling in those details! |
We're now using npm consistently, the AWS CLI is mentioned in the deploy docs, and I added some documentation on how i18n works. Is the README clearer now? |
Yeah I think so! The main reason why you'd want to set up that google sheet would be to assist in translating content, right? In this case it would be if you wanted to re-translate existing copy? Does the script replace everything that is already in the generated files? That might be a good thing to be explicit about; I assume for most cases you'd probably not want to re translate everything but just replace some content around the name of a city, etc. Good note about the list of supported languages; I had made a note of checking into that. I reckon some orgs might want to slowly roll out language support depending on their needs or capacity. |
This env value doesn't appear to be referenced anywhere else?
chi-covid-resources/Makefile
Line 21 in 68a79a3
On the subject of the Makefile, any particular reason why the project uses npm but the deploy process uses yarn? Would it be useful to simplify it to use npm for both? (I reckon this may be a holdover from when npm was much worse at locking dependencies, etc)
It also appears the Makefile relies on the aws CLI, which is a dependency not currently documented
Actually the more I think about it and look at the Makefile, we could probably get rid of it and replace it with a command inside the package.json that uses the deploy command with the config variables used via npmrc or something similar.
The text was updated successfully, but these errors were encountered: