Skip to content

Remove unused pycountry dependency.#3580

Merged
bjester merged 1 commit intolearningequality:unstablefrom
rtibbles:no_pycountry_for_old_men
Aug 29, 2022
Merged

Remove unused pycountry dependency.#3580
bjester merged 1 commit intolearningequality:unstablefrom
rtibbles:no_pycountry_for_old_men

Conversation

@rtibbles
Copy link
Copy Markdown
Member

Summary

Description of the change(s) you made

  • We were adding pycountry to our locale paths
  • A quick grep of the code base showed no usage of _(...) gettext that would require strings that don't already exist in Django or are our own internal strings.

Manual verification steps performed

Check that the server still runs fine.
Check that all language names are properly translated in the frontend language changing interfaces.

@rtibbles rtibbles requested a review from bjester August 24, 2022 23:54
@bjester
Copy link
Copy Markdown
Member

bjester commented Aug 25, 2022

I don't really understand the significance of adding pycountry to the locales, but regarding

A quick grep of the code base showed no usage of _(...) gettext that would require strings that don't already exist in Django or are our own internal strings.

What about template translations? They use gettext internally

@rtibbles
Copy link
Copy Markdown
Member Author

Can double check!

@rtibbles
Copy link
Copy Markdown
Member Author

I don't really understand the significance of adding pycountry to the locales

As far as I can tell from the pycountry repo, the locale files contain internationalized names for countries and languages. We use Django's internal naming registry for language names to do this for interface languages, and le-utils names for content languages, so I am sure we're not leveraging it for displaying language names anywhere.

@rtibbles
Copy link
Copy Markdown
Member Author

What about template translations?

Have double checked all the template translations too, none there that are not regular interface text (no country or language names).

Copy link
Copy Markdown
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

LGTM

@bjester bjester merged commit 2809cf8 into learningequality:unstable Aug 29, 2022
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.

2 participants