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

Is google sheet referenced in Makefile still required for this project? #1

Open
easherma opened this issue May 6, 2020 · 4 comments

Comments

@easherma
Copy link
Contributor

easherma commented May 6, 2020

This env value doesn't appear to be referenced anywhere else?

wget -O $@ 'https://docs.google.com/spreadsheets/d/${SPREADSHEET_ID}/export?format=csv&id=${SPREADSHEET_ID}&gid=0'

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.

@pjsier
Copy link
Contributor

pjsier commented May 7, 2020

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

@easherma
Copy link
Contributor Author

easherma commented May 7, 2020

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!

@pjsier
Copy link
Contributor

pjsier commented May 11, 2020

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?

@easherma
Copy link
Contributor Author

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.

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

2 participants