A long time ago, Koji wrapped most or all strings with a _() method. I had to research this because I didn't know what this was about.
Turns out it's a convention for doing localization, using gettext. This blog explains more: https://danishpraka.sh/2018/09/09/localization-python.html . There's a Python script helper ("pygettext") for indexing all those _() calls in a codebase and writing the English strings into a .pot template for translators to complete.
I think we should remove this _() method for the following reasons:
1. I can't find that we ever had .pot or .po files for Koji's Git history. We never had localization for Koji, and I don't think it makes sense to try to do this now, particularly for the CLI.
2. In recent years, we've stopped wrapping new strings with _() as we introduce them. Now it's an additional hurdle to new contributors: "what is this thing", and "should I wrap strings or not?" Maybe one rule is: don't try to do it anywhere but the CLI? But then why is it used in koji-gc or koji-shadow? git grep "def _(" is also confusing - why is the same method defined in four different places?
3. In some cases the CLI prints the hub exception's faultString directly to the console (for example: "koji disable-user" for a user that does not exist). Those messages will always be unlocalized (a good thing, since those messages are evolving into part of Koji's RPC protocol now as 3rd party apps begin to scrape faultString, https://github.com/ktdreyer/koji-ansible/issues/221).
4. There are just weird tech debt things / code smell things associated with this, like https://pagure.io/koji/pull-request/1260 or the fact that we have to write "pragma: no cover" in koji/__init__.py because it's too hard to test.
Koji's README.md describes the client as thin; let's make it even thinner and remove the _() method from Koji altogether.
If external plugins or scripts are still calling koji._() or cli.koji_cli.lib._(), we can make the method raise a DeprecationWarning.
- Ken
Seems reasonable to me. We included those at the very beginning because it seemed like "the right way" and we thought we'd eventually have localization.
On Tue, Oct 12, 2021 at 12:56 PM Ken Dreyer ktdreyer@ktdreyer.com wrote:
A long time ago, Koji wrapped most or all strings with a _() method. I had to research this because I didn't know what this was about.
Turns out it's a convention for doing localization, using gettext. This blog explains more: https://danishpraka.sh/2018/09/09/localization-python.html . There's a Python script helper ("pygettext") for indexing all those _() calls in a codebase and writing the English strings into a .pot template for translators to complete.
I think we should remove this _() method for the following reasons:
- I can't find that we ever had .pot or .po files for Koji's Git
history. We never had localization for Koji, and I don't think it makes sense to try to do this now, particularly for the CLI.
- In recent years, we've stopped wrapping new strings with _() as we
introduce them. Now it's an additional hurdle to new contributors: "what is this thing", and "should I wrap strings or not?" Maybe one rule is: don't try to do it anywhere but the CLI? But then why is it used in koji-gc or koji-shadow? git grep "def _(" is also confusing - why is the same method defined in four different places?
- In some cases the CLI prints the hub exception's faultString
directly to the console (for example: "koji disable-user" for a user that does not exist). Those messages will always be unlocalized (a good thing, since those messages are evolving into part of Koji's RPC protocol now as 3rd party apps begin to scrape faultString, https://github.com/ktdreyer/koji-ansible/issues/221).
- There are just weird tech debt things / code smell things
associated with this, like https://pagure.io/koji/pull-request/1260 or the fact that we have to write "pragma: no cover" in koji/__init__.py because it's too hard to test.
Koji's README.md describes the client as thin; let's make it even thinner and remove the _() method from Koji altogether.
If external plugins or scripts are still calling koji._() or cli.koji_cli.lib._(), we can make the method raise a DeprecationWarning.
- Ken
koji-devel mailing list -- koji-devel@lists.fedorahosted.org To unsubscribe send an email to koji-devel-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/koji-devel@lists.fedorahosted.o... Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
Agreed, I've created https://pagure.io/koji/issue/3072 for that (in case of no resources we can push it to later release).
On Wed, Oct 13, 2021 at 3:13 AM Michael McLean mikem@redhat.com wrote:
Seems reasonable to me. We included those at the very beginning because it seemed like "the right way" and we thought we'd eventually have localization.
On Tue, Oct 12, 2021 at 12:56 PM Ken Dreyer ktdreyer@ktdreyer.com wrote:
A long time ago, Koji wrapped most or all strings with a _() method. I had to research this because I didn't know what this was about.
Turns out it's a convention for doing localization, using gettext. This blog explains more: https://danishpraka.sh/2018/09/09/localization-python.html . There's a Python script helper ("pygettext") for indexing all those _() calls in a codebase and writing the English strings into a .pot template for translators to complete.
I think we should remove this _() method for the following reasons:
- I can't find that we ever had .pot or .po files for Koji's Git
history. We never had localization for Koji, and I don't think it makes sense to try to do this now, particularly for the CLI.
- In recent years, we've stopped wrapping new strings with _() as we
introduce them. Now it's an additional hurdle to new contributors: "what is this thing", and "should I wrap strings or not?" Maybe one rule is: don't try to do it anywhere but the CLI? But then why is it used in koji-gc or koji-shadow? git grep "def _(" is also confusing - why is the same method defined in four different places?
- In some cases the CLI prints the hub exception's faultString
directly to the console (for example: "koji disable-user" for a user that does not exist). Those messages will always be unlocalized (a good thing, since those messages are evolving into part of Koji's RPC protocol now as 3rd party apps begin to scrape faultString, https://github.com/ktdreyer/koji-ansible/issues/221).
- There are just weird tech debt things / code smell things
associated with this, like https://pagure.io/koji/pull-request/1260 or the fact that we have to write "pragma: no cover" in koji/__init__.py because it's too hard to test.
Koji's README.md describes the client as thin; let's make it even thinner and remove the _() method from Koji altogether.
If external plugins or scripts are still calling koji._() or cli.koji_cli.lib._(), we can make the method raise a DeprecationWarning.
- Ken
koji-devel mailing list -- koji-devel@lists.fedorahosted.org To unsubscribe send an email to koji-devel-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/koji-devel@lists.fedorahosted.o... Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
koji-devel mailing list -- koji-devel@lists.fedorahosted.org To unsubscribe send an email to koji-devel-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/koji-devel@lists.fedorahosted.o... Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
koji-devel@lists.stg.fedorahosted.org
-
Ken Dreyer -
Michael McLean -
Tomas Kopecek