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

Diagnostic/ add new ell translations #12087

Merged
merged 6 commits into from
Jul 17, 2024
Merged

Conversation

eadams17
Copy link
Member

@eadams17 eadams17 commented Jul 16, 2024

WHAT

add new ell translations for the new ELL Diagnostics

WHY

we have new instruction and cue translations

HOW

update all of the translation JSON files with the new key-value pairs

Screenshots

(If applicable. Also, please censor any sensitive data)
Screen Shot 2024-07-16 at 2 30 32 PM

Notion Card Links

(Please provide links to any relevant Notion card(s) relevant to this PR.)
https://www.notion.so/quill/Add-New-ELL-Diagnostic-Translations-20f9eb0d6fea4aab83f2fd617ef9dbc4?pvs=4

What have you done to QA this feature?

(Provide enough detail that a reviewer could assess whether additional QA should be done. For larger projects, additionally use the Engineer Feature Testing Notion template. Review Guidelines if needed: https://www.notion.so/quill/Github-PR-QA-Guidelines-49e99fc965654ceeb8c6249bd9d181d7)
clicked through each question of each new diagnostic for each language on staging (v tedious)

PR Checklist Your Answer
Have you added and/or updated tests? no-- manually tested (JSON file updates)
Have you deployed to Staging? yes
Self-Review: Have you done an initial self-review of the code below on Github? yes

Copy link
Contributor

@samanthamjohn samanthamjohn left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Member

@emilia-friedberg emilia-friedberg left a comment

Choose a reason for hiding this comment

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

This looks good! I don't think we need to do this now, especially because this is time-sensitive, but at some point it might be nice to do some clean up where we make all those keys into constants since they're duplicated across all the files.

@eadams17 eadams17 merged commit 3f28eb4 into develop Jul 17, 2024
16 checks passed
@eadams17 eadams17 deleted the add-new-ell-translations branch July 17, 2024 19:32
eadams17 added a commit that referenced this pull request Jul 17, 2024
Diagnostic/ add new ell translations (#12087)
Copy link
Contributor

Choose a reason for hiding this comment

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

@eadams17 looks like this was a mistake!

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

Successfully merging this pull request may close these issues.

None yet

3 participants