flattenthecurve / guide

https://www.flattenthecurve.com
Creative Commons Attribution 4.0 International
38 stars 33 forks source link

Reorganized What to Do #336

Closed emersonthis closed 4 years ago

emersonthis commented 4 years ago

Eventually closes #294

Done (English only)

Not done (yet)

"But you removed the red and green colors from the titles! Booooo! Why???" 😫 There are a few reasons why:

Screen Shot 2020-04-08 at 9 41 11 PM
emersonthis commented 4 years ago

@mverzilli @nditada If you give me a thumbs up on this, I'll loop back and update the other language files as much as I can before removing the WIP label.

github-actions[bot] commented 4 years ago

This pull request is being automatically deployed with now-deployment

Built with commit 9d7863cabd76e0264e26bb7d99268f2c7e21e2cb

✅ Preview: https://guide-preview-bufnmlwz1.now.sh

github-actions[bot] commented 4 years ago

This pull request is being automatically deployed with now-deployment

Built with commit 979d3054fb6b847b9a6687a2b56ceacccf0688e6

✅ Preview: https://guide-preview-5gokj2wom.now.sh

jmcmurry commented 4 years ago

From a content perspective, this all looks great. I can't speak to the technical aspect as changing 64 docs at once is hard to track the impact of. Is there a staging area we can view these in?

nditada commented 4 years ago

@emersonthis this looks good to me from a structure standpoint and @jmcmurry gave her ok in terms of content, so we are good to go. Did you mean this PR as a WIP or shall we merge and then adapt the other languages?

emersonthis commented 4 years ago

@nditada I'd vote for merging as soon as possible, but let me know if there's specific things I can do to make the translators' job easier. If not, let's merge.

github-actions[bot] commented 4 years ago

This pull request is being automatically deployed with now-deployment

Built with commit a02eb99618242a2c5c7dd3e531b16e044b699a00

✅ Preview: https://guide-preview-luovjzgxo.now.sh

github-actions[bot] commented 4 years ago

This pull request is being automatically deployed with now-deployment

Built with commit ac83454e3ae0107c24e8d0c210b6548c2b8e403f

✅ Preview: https://guide-preview-bpj9miuzy.now.sh

emersonthis commented 4 years ago

@jmcmurry I added the flu buddy back in.

I think this might be ready to merge.

emersonthis commented 4 years ago

Is there a staging area

@jmcmurry Yes. There’s a link above to a deployed preview of this PR

jaysonvirissimo commented 4 years ago

Not sure if we have an actual QA process, but things LGTM on the https://guide-preview-bpj9miuzy.now.sh deploy. 👍

emersonthis commented 4 years ago

I agree. I think this can get merged but I want @nditada to endorse because I don’t understand the implications for translations well enough to feel confident merging this myself. If someone can speak on this that would work too.

rousik commented 4 years ago

@emersonthis I have tried to integrate your changes and rerun import_languages.rb on DE and CS languages. As you can see here, this doesn't quite work well: https://guide-preview-gqdakh65n.now.sh/de/act-and-prepare/

I have not yet looked into why is it happening but it seems not safe as-is.

rousik commented 4 years ago

Your changes combined with the language refresh for de,cs languages can be found in the branch preview/issue-294-translations

rousik commented 4 years ago

The problem seems to be due to large number of untranslated keys (10 per language):

./_sections/act_and_prepare/cs/05-reduce-stress/00-tips-to-reduce-stress.md
./_sections/act_and_prepare/cs/04-support-community/15-donate.md
./_sections/act_and_prepare/cs/04-support-community/00-how-to-support-your-community.md
./_sections/act_and_prepare/cs/02-household-practices/05-shop-carefully.md
./_sections/act_and_prepare/cs/02-household-practices/06-pandemic-pal.md
./_sections/act_and_prepare/cs/02-household-practices/00-household-practices.md
./_sections/act_and_prepare/cs/01-not-infected/02-avoid-crows.md
./_sections/act_and_prepare/cs/01-not-infected/00-how-to-not-get-infected.md
./_sections/act_and_prepare/cs/01-not-infected/01-stay-home.md
./_sections/act_and_prepare/cs/03-if-sick/00-what-to-do-if-you-get-sick.md

My understanding is that basenames of markdown files are used to construct Lokalise keys (e.g. stay-home). If you move file into another directory or change its numerical prefix, that should not be a problem, but changing filename itself will dissociate it from the existing translations.

I can see that this is happening for some files listed above, e.g.:

shop-when-at-risk -> shop-carefully
stay-connected -> avoid-crows

The simplest interim solution to this problem would be to retain original naming wherever possible. Maybe there is a way to elegantly update Lokalise keys but I don't know about it yet.

The second problem is due to new content (h2 files for each section). I think for this we will simply need to kick off Lokalise task once your change is merged into head (I think that Lokalise is pulling keys out of master branch continuously).

emersonthis commented 4 years ago

@rousik i mostly understand everything you wrote. What needs to happen next in your opinion? The challenge for me right now is that I don’t know how much to “weigh” the issues for translations against the improvements to the English version. I don’t want to cause translation problems but it’s not clear whether some translations are going to catch up no matter what we do... and in the mean time the English version of this page is not very accessible. I don’t know how to make his call. @nditada help ;-)

emersonthis commented 4 years ago

Another factor to consider is that some of the translations that will “break” are for older versions of this content that need to be updated anyway. This PR was supposed to be a structural change before editing a bunch of these items for content improvements.

rousik commented 4 years ago

I think your reasoning is sound. Deploying this and then kicking off translations seems like a good order of operations. The translated pages should remain unaffected until after we pull the new strings from Lokalise so we should be safe until then.

If I'm reading this Lokalise documentation right, there may be a way to link/group the duplicate keys which might allow us to associate these new strings with existing translations and that might reduce the ops load on the translators.

emersonthis commented 4 years ago

Ok. I saw a 👍 from @nditada earlier and it sounds like you're on board. So I'm going to merge this. It'll take me a few minutes fix the merge conflicts (again) so if anyone has strong objections this is "last call" as they say...

github-actions[bot] commented 4 years ago

This pull request is being automatically deployed with now-deployment

Built with commit 31c2b51af4b2beb9efffbf725de9fc86b9094ff7

✅ Preview: https://guide-preview-cd0pakrjz.now.sh

emersonthis commented 4 years ago

I spoke too soon... I need at least one reviewer to approve before I can pull the trigger on merging this.

rousik commented 4 years ago

I will take a look at this tomorrow morning (Mountain Time) and once you merge this I will try to fix up our translation strings in Lokalise.

On Wed, Apr 22, 2020 at 9:46 PM emersonthis notifications@github.com wrote:

@emersonthis https://github.com/emersonthis requested your review on:

336 https://github.com/flattenthecurve/guide/pull/336 Reorganized What

to Do.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/flattenthecurve/guide/pull/336#event-3263254300, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYSCGDS73HHQCPL2B4UAC3RN62ZRANCNFSM4MEN6QVA .

emersonthis commented 4 years ago

@rousik thanks! This is a busy week for me, but let me know if I can help with the translation strings

rousik commented 4 years ago

Turns out that the changes to the content along with the change of filenames makes duplicate finder not work. I will look into whether it's possible to link those things or pre-populate new translations at least.

On Thu, Apr 23, 2020 at 9:03 AM emersonthis notifications@github.com wrote:

@rousik https://github.com/rousik thanks! This is a busy week for me, but let me know if I can help with the translation strings

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flattenthecurve/guide/pull/336#issuecomment-618448593, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYSCGFCXQ7C6ERDXEBOB2TROBKFXANCNFSM4MEN6QVA .