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

docs: add OEP-0058 - Translations Management #367

Merged

Conversation

Carlos-Muniz
Copy link
Contributor

@Carlos-Muniz Carlos-Muniz commented Aug 8, 2022

OEP-58

This PR introduces OEP-58 for Translations Management - Using the GitHub Transifex App and a separate repository containing translation files as a replacement for the edx-transifex-bot, and the current organizational structures used in the Transifex project edx-platform.

The proposal describes reasons for establishing this new process for managing our Translations by creating a new repository, placing all translation files in this repository, and using the Transifex Github App to manage the syncing of these Translation files as they are translated by the Open edX community.

Update:
A 3-part prototype was created for the openedx/credentials repository in the openedx/openedx-translations repository.

  1. A GitHub action that generates English translation source files from the credentials repository, and adds them to the openedx-translations repository.
  2. The transifex.yml file that allows for syncing the openedx-translations repository and an openedx-translations project in Transifex.
  3. A CLI tool in openedx/openedx-atlas that can be used to pull translation files from openedx-translations back into a repository.

@Carlos-Muniz
Copy link
Contributor Author

@sarina @feanil Who should the Arbiter be?

@Carlos-Muniz Carlos-Muniz changed the title docs: add OEP - Translations Management docs: add OEP 58 - Translations Management Aug 8, 2022
@feanil feanil changed the title docs: add OEP 58 - Translations Management docs: OEP-58 - Translations Management Aug 8, 2022
@Carlos-Muniz Carlos-Muniz changed the title docs: OEP-58 - Translations Management docs: add OEP-58 - Translations Management Aug 8, 2022
@nedbat
Copy link
Contributor

nedbat commented Aug 9, 2022

I will be the arbiter.

Copy link
Contributor

@sarina sarina left a comment

Choose a reason for hiding this comment

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

Good start.

From OEP-1:

A Process proposal describes a change to how the Open edX community functions.

A Best Practice proposal describes a technology or implementation choice that the Open edX community believes all applicable Open edX services and/or libraries should use or follow.

An Architecture proposal describes a concrete design problem with several potential solutions and the rationale behind the decision. The decision may be specific to a significant Open edX feature or a cross-cutting technical need.

You've specified this is a Process proposal, but those are typically... well, process documents - like Maintainers, DEPR, or "How OEPs Work". This feels like an architecture proposal.

oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
oeps/processes/oep-0058-proc-translations-management.rst Outdated Show resolved Hide resolved
Carlos Muniz and others added 7 commits August 10, 2022 10:17
New title is now: "Translations Management", which matches the file
name.
Also updates the last modified section.
Co-authored-by: Sarina Canelake <sarina@tcril.org>
Co-authored-by: Sarina Canelake <sarina@tcril.org>
Co-authored-by: Sarina Canelake <sarina@tcril.org>
In addition, the first bullet point and sub-bullet point were combined
to create a stronger first point.
@Carlos-Muniz Carlos-Muniz force-pushed the Carlos-Muniz/translations-management branch from 958aeae to a72f901 Compare August 10, 2022 20:33
Carlos Muniz added 3 commits August 10, 2022 16:34
A new section has been added named "Proposed Implementation". This
section contains the content from 3 sections: "Next Steps",
"Consequences", and "Locations". The "Next Steps" section header was
removed, and its content place directly under "Proposed Implementation".
The "Consequences" Section was renamed to "Pros & Cons".
@Carlos-Muniz
Copy link
Contributor Author

@sarina Thank you for your reviews. I've made a lot of good changes based on your comments.

Re: "Next Steps" - It's not very common, only in OEP-19 and OEP-22. But in the reformatting, I decided to stick that content under "Proposed Implementation".

@sarina
Copy link
Contributor

sarina commented Aug 12, 2022

@Carlos-Muniz I think I've realized I don't love the "Next Steps" section because OEPs are meant to be long-term documents that are references for years; putting in "Next Steps" means people who read it later have no idea what is complete.

For OEP-54, I did not define the types of core contributors within the OEP itself (as we update OEPs sparingly). I defined the roles instead on a wiki page, and linked the wiki page from OEP-54. Perhaps it would be better to create a "project plan" wiki page that can be updated frequently over the implementation period.

@jesgreco
Copy link

jesgreco commented Nov 9, 2022

@Carlos-Muniz I really like the structure and the content of each section of this document!!! I have some little stuff for you to review specified by line:

• Line 40: "English" should be in capital letters.
• Line 61: edx-transifex-bot should be all in small case, to maintain parallelism like lines 67, 68.
• Line 93: Add a full stop.
• Line 127: This link is not working:  https://github.com/openedx/openedx-translation (at least, for me).
• Line 212: "Docker" should be in capital letters.

Thanks!

@Carlos-Muniz Carlos-Muniz force-pushed the Carlos-Muniz/translations-management branch from 602b826 to 7a06a16 Compare November 9, 2022 18:56
@Carlos-Muniz
Copy link
Contributor Author

@jesgreco thank you for your comments! I've incorporated the majority of your notes.

* - Arbiter
- Ned Batchelder <ned@edx.org>
* - Status
- Under Review
Copy link
Contributor

@robrap robrap Nov 9, 2022

Choose a reason for hiding this comment

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

Based on the Provisional definition and the docs for status changes, I'd recommend:

  1. Updating the status to Under Review (=> Provisional) to clarify the intended status upon merge, assuming that status is agreed upon. And,

  2. Adding a Follow-up Work page to this header as noted in the docs above. This link might also provide some thoughts on how this work will come to be.

  3. Additionally, this isn't a blocker to this OEP, but is related to "Follow-up Work". I'm wondering what thought has been given to how companies will need to handle private repos and translations? Maybe a separate Discuss thread could be started for this, but has thought already been given to this? Is the idea that others could easily replicate this pattern and re-use tooling against a private repo of translations? How much work will that involve? Etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your comments @robrap.

  1. If I understand correctly, you want to clarify that the next status after "Under Review" should be "Provisional" and not "Accepted", because it hasn't been vetted and adopted in the platform. This makes sense to me, but I don't think we need to specify this in the document.

  2. I will add a link named “Follow-up Work” to the References section of the OEP header.

  3. I think this is a good first question for the Follow-up Work link. The openedx-atlas CLI tool can pull translation files from any repository that works with git. And the GitHub Action that generates translations and moves them to openedx-translations can also be reappropriated for private repos. The work involved will be: copying the github action that generates the translations to another repository, changing the organization and names of the repos, and attaching it to a separate Transifex project via the app.

Copy link
Member

Choose a reason for hiding this comment

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

@Carlos-Muniz on point 1, @robrap is suggesting that you change the status from:

  • Under Review

to one of these:

  1. Under Review (=> Provisional)
  2. Under Review (=> Accepted)

in order to clarify whether you want this to merge in as a Provisional or Accepted, which is information the reviewers should know. I agree with him.

Copy link
Member

Choose a reason for hiding this comment

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

Aside, @robrap : I think this is evidence that the OEP-1 status guidance that we collaborated on could be improved :) OEP-1 presents Under Review as a valid status even though we'd really prefer that folks always include the (=> ...) part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the clarification. I've changed it.

@jansenk
Copy link

jansenk commented Nov 10, 2022

I'm curious about the use cases for repos that currently use the transifex bot and are installed as dependencies to edx-platform (specifically https://github.com/openedx/edx-ora2, but the use case in general.)

When we release a new version of the xblock or merge to master, we still need to update edx-platform to install the newest version. If we updated translations from ora2 on merge, or even on release, that doesn't necessarily mean that edx-platform is running a version that is compatible with those updated translation files, until the version is updated.

@jansenk
Copy link

jansenk commented Nov 10, 2022

I'm curious about the use cases for repos that currently use the transifex bot and are installed as dependencies to edx-platform (specifically https://github.com/openedx/edx-ora2, but the use case in general.)

I've thought about this overnight and I think my confusion revolves around the "dummy" translations that django generates before transifex fills in the real translations.

Previously, a developer would run the django command to generate updated translations files, including a dummy translation of any new user-facing strings. the transifex-bot would then separately upload these translation files to transifex, then pull down and merge the "real" translations.

Am I correct in interpreting this change as being intended to replace both of these steps? A github action will generate the translation files in the new repo, the transifex app will update to the real translations in the new repo through the github app, and then atlas will somehow insert the real translations back into the target repo? Or, will it just be intended to replace the transifex-bot portion? Developers still generate new translation files with dummy translations, and then eventually atlas merges in the "real" translations from transifex?

placement of the translation files kept in openedx-translations into locally cloned
repositories for development and containers containing the code translation files are
formed from. This tool will manage the placement of translation files through an editable
atlas configuration file (atlas.yml) kept in the repositories that have
Copy link

Choose a reason for hiding this comment

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

This may be an implementation detail, but I'm wary of configuration files. If there is a hardcoded atlas.yml file in a repo, then it becomes difficult to use settings that are different from the defaults. For instance, if a user wants to download translation strings for a language that is not present in edx-platform (a very common request) they will have to modify atlas.yml, and thus fork edx-platform. Nobody wants that.

Instead, I recommend to implement good defaults for openedx-atlas, and to make it possible to override defaults easily via the CLI. For instance, by default openedx-atlas could download strings for all languages with 50%+ verified rate (as i18-tool currently does) and we could add extra languages or modify the minimum required rate from the CLI.

Furthermore, my experience with openedx-i18n makes me doubt whether a CLI tool is even necessary. In Tutor translation strings are simply downloaded and moved to a dedicated directory with a single curl command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

openedx-atlas already provides the ability to override default parameter values and an atlas.yml configuration file by using the CLI's flags.

atlas pull -r "openedx/openedx-translations" -b "main" -d "credentials"

This would pull translation files from the credentials directory in the main branch
from the repository openedx/openedx-translations.

There are many other features that could be beneficial, such as the ones you described in your comment. We can hash out these implementation details further in Follow-up Work.

Copy link
Member

Choose a reason for hiding this comment

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

semi-related thread: #367 (comment)

*********

Dumps of the translation/localization files from Transifex for the Open edX Release
project already exist in a repository with the name of openedx/openedx-i18n. A new
Copy link

Choose a reason for hiding this comment

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

If the new openedx-translations repo contains strings for all languages and repos and it follows the regular minor/major release schedule, then yes we will be able to get rid of the openedx-i18n repo. I'm looking forward to it :)

repository `edx-platform`_ located at
``edx-platform/conf/locale/en/LC_MESSAGES/django.po`` would be moved to the new
repository with the name openedx-translations_ located at
``openedx-translation/edx-platform/conf/locale/en/LC_MESSAGES/django.po``. For easier
Copy link

@regisb regisb Nov 11, 2022

Choose a reason for hiding this comment

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

Can I suggest to create an intermediate "repos" folder, such that we have openedx-translations/repos/edx-platform? Otherwise we will have a lot of repos located at the root, as we already do: https://github.com/openedx/openedx-translations This will quickly become unwieldy. And if we need extra translations-specific folders at the root (e.g: "docs"), they will be mixed with repo names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good idea that we can implement once the review period is complete.

@Carlos-Muniz
Copy link
Contributor Author

Am I correct in interpreting this change as being intended to replace both of these steps? A github action will generate the translation files in the new repo, the transifex app will update to the real translations in the new repo through the github app, and then atlas will somehow insert the real translations back into the target repo? Or, will it just be intended to replace the transifex-bot portion? Developers still generate new translation files with dummy translations, and then eventually atlas merges in the "real" translations from transifex?

@jansenk Thank you for your comments!

To clarify the process:

  1. A GitHub Action will generate the translation source (English) files for a repository, and commit them to openedx-translations.
  2. The Transifex GitHub app will upload them to Transifex.com
  3. Translators will translate + review the translations.
  4. Once the translations hit an approved completion percentage, the translation files will be committed back to openedx-translations.
  5. The openedx-atlas CLI tool can be used during development or deployment to pull (via git sparse-checkout) the translation files from openedx-translations that correspond with a locally downloaded repo you are developing on, or similarly, to a repository that is being turned into a docker image.

When we release a new version of the xblock or merge to master, we still need to update edx-platform to install the newest version. If we updated translations from ora2 on merge, or even on release, that doesn't necessarily mean that edx-platform is running a version that is compatible with those updated translation files, until the version is updated.

In order to fix this issue, we can add a feature of openedx-atlas CLI tool that is able to pull a specific commit from openedx-translations, along with a specific branch and directory. This way you can fully customize which translation files you want to use, instead of using the latest version or a version tied to a named open release.

This signifes that after the review period, the status will change to
Provisional and questions related to the implementation of this OEP-58
in order to reach the Accepted status will be answered in the Follow-up
Work page linked here.
@Carlos-Muniz Carlos-Muniz force-pushed the Carlos-Muniz/translations-management branch from 9c84437 to e86e67d Compare November 14, 2022 17:29
Copy link
Contributor

@pomegranited pomegranited left a comment

Choose a reason for hiding this comment

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

👍 Glad to see the "Follow-up" document linked -- there's sure to be more implementation details that need to be sorted out as this is implemented, but the concept is great. I'm happy for this to move to Provisional / Accepted.

Thank you for your excellent work here @Carlos-Muniz @feanil @sarina !

  • I read through the this change.
  • I considered the procedural and security implications of this change.

Copy link
Contributor

@pshiu pshiu left a comment

Choose a reason for hiding this comment

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

Amazing work on the example repos. A couple of questions on approach!

maintenance burden and are more future proof of changes they might make since they
maintain both the API and the `Transifex GitHub App`_.

Rationale for consolidating translations files centrally
Copy link
Contributor

@pshiu pshiu Nov 15, 2022

Choose a reason for hiding this comment

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

Is anything changing about what powers or responsibilities repository maintainers have over their strings? For example:

  • Who would have the power/responsibility to review/approve translations for each repo?
  • Will maintainers also have power/responsibility over their repo's folder within openedx-translations?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious:

(1) What are you asking for?
(2) What "powers" do you want and why?
(3) What "powers" do you currently exercise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Translation WG is in charge of translating and reviewing translations, and to my knowledge these translations are just automatically merged in, e.g. openedx/paragon#1744. With the changes proposed in OEP-58, Translations WG would still be in charge of translating and reviewing translations. Approval limits (once it hits X% translated it is automatically committed to openedx-translations) can be set per translation file. I can see a situation where maintainers request the Translation WG to set the approval limit below the standard set by the Translations WG. That being said, our Translators are top knotch and keep translations at 90% or higher.

I think answers to @sarina's questions can help shed light to what type of input maintainers are looking for.

Copy link
Contributor

Choose a reason for hiding this comment

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

What are you asking for?

I am trying to understand what the interaction should be between Open edX developers and/or maintainers with the Translation WG. For example, I currently do not know where to find information on the roles and responsibilities of maintainers, developers, and the Translation WG. It may be helpful to point to that information in this OEP.

What "powers" do you want and why? What "powers" do you currently exercise?

The Translation WG is in charge of translating and reviewing translations

This is new to me! I greatly appreciate the Translation WG's work, don't get me wrong. However, I thought the status quo was that maintainers ultimately owned their repo's translations. For example, I think many maintainers were routinely given Project Maintainer access to Transifex, and would frequently help add/change/remove strings or review translations. If the ownership of translations is changing or has already changed, it may be helpful to point to that change in this OEP.

Also, previously, maintainers were able to modify their translation files. Now, via this OEP, it appears those translation files are being moved to a centralized repository, and I am unsure who will own that repository and if any of the access maintainers previously had to change those files will change. In short, "will there ever be non-bot PRs for the translation files in openedx-translations?" and "who will approve and/or merge those PRs?".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pshiu brought up some important questions.

  • Who will maintain the openedx-translations repository?
  • How should errors/bugs in translations in the openedx-translations repository be fixed?
  • Who is in charge of approving and/or merging PRs in the openedx-translations repository?

It may be unreasonable to expect any one group to maintain openedx-translations as there are lots of stakeholders. Translators are expected to translate, but may not get to see if their translations will cause errors. Developers may see that there are errors, but may not understand the language of the translation. There will have to be cooperation.

We should set up an issue request form on openedx-translations (much like the Github Requests in tcril-engineering) that notifies the Translations WG on slack when there is a translation error that needs to be fixed. This way, Translators can be notified of an error in a way they will be able to digest it, and Developers can write the problem and proposed solution in a way that is common to them.

We will have to look further into how the GitHub Transifex app manages changes to translation files outside of Transifex (openedx/openedx-translations#29) to confirm that making PRs is a possible fix that developers can make. In the case that it is allowed, we can treat it as any other repository and nominate Open edX Community members to be Core Contributors to openedx-translations based on their technical experience and participation in the Translations WG.

@brian-smith-tcril is currently looking at this issue and will update with the Transifex App's capability.

Copy link
Contributor

@brian-smith-tcril brian-smith-tcril Nov 17, 2022

Choose a reason for hiding this comment

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

i commented on the issue on the openedx-translations repo, but i'll reiterate here:

based on transifex docs (and @Carlos-Muniz and my testing of the Transifex github app integration)

For existing resources that have already been synced with GitHub, if translations are found in GitHub, they will be sent to Transifex only if no translations are found for that language in Transifex

knowing this, it seems we should stick to the idea of transifex as a source of truth for all translated strings, and any updates should happen in transifex

Comment on lines 189 to 191
will also support selecting which languages to be included in an Open edX deployment. The
tool will have to be used/ran as part of the setup of a repository, whether for
development or deployment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Adapting our development & deployment tooling seems like one of the biggest impact areas of this OEP. Has any research been done in regards to how openedx-atlas will be integrated into Open edX's supported development & deployment environments (devstack, Tutor, tubular, edx_ansible), and what work it will take?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since openedx-atlas works as a configurable wrapper to git's sparse-checkout, openedx-atlas can be used to pull translations from openedx-translations right after git clone is used to pull the repository from GitHub. This should cover most supported development & deployment environments, though I'm curious if there are any others that may not work like this.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should cover most supported development & deployment environments, though I'm curious if there are any others that may not work like this.

Having atlas built in python adds complexity to the deployment of MFEs. It seems there are a few paths that we could go down

The first option would be to keep atlas in python, and to use the python implementation everywhere

Pros:
* Flexibile/Extensibile
* Easy to maintain (only need to maintain one implementation)
Cons:
* Requires python in build/deployment envs

Another option would be to write atlas in bash, that way python would not be required in build/deployment environments for MFEs.

Pros:
* Runs (pretty much) everywhere
Cons:
* Bash can get messy when projects get complex

A third option would be to also write atlas functionality in js and package for NPM so MFEs (or frontend-build) could import it.

Pros:
* Easy integration for MFEs
* Flexible/extensible
Cons:
* Requires maintaining duplicated functionality (need to implement everything in both js and python)

If the scope of atlas stays limited, i'd lean towards the bash option. If there's an expectation of increased complexity within atlas, however, there are definite benefits to using full fledged programming languages.

Copy link
Member

@kdmccormick kdmccormick Nov 15, 2022

Choose a reason for hiding this comment

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

@bmtcril mentioned a fourth option: get rid of openedx-atlas, and publish releases of .tar.gz/.zip files from the openedx-translations repository

Pros:
* No sparse checkout logic needed, no separate CLI tool needed.
* All the complexity would be in the openedx-translations repo; other repos would just download & unpack the .tar.gzs.
Cons:
* Would need to add automation to the openedx-translations repo to regularly publish these
* The .tar.gzs would contain every language (or every supported language?)
  * It's unclear whether the .tar.gzs would be too big or not.
     edx-platform's locale directory is currently 60 Mb, gzips down to 14 Mb.
* Is there other stuff we think openedx-atlas would do that we'd be leaving out with this option?

Copy link
Contributor

Choose a reason for hiding this comment

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

Solely from the MFE repo perspective, I'm for gzips. Github workflows for publishing them on every commit would be pretty simple. And I very much doubt download sizes will be problematic. For all I know tx pull could be even less efficient. 🤷🏼

What would stop us from storing translations per-repo in opendx-translations, though? That would make each individual pull pretty tiny.

If that doesn't work, for whatever reason, I'd vote for bash.

Copy link
Member

Choose a reason for hiding this comment

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

@arbrandes

What would stop us from storing translations per-repo in opendx-translations, though? That would make each individual pull pretty tiny.

That is indeed the suggestion on the table -- one gzip published per repo 🙂 It's just that each gzip would have every language unless we wanted to publish R*L gzips, where R is the number of repos and L is the number of languages.

community member with exclusive access to the legacy infrastructure currently running the
edx-transifex-bot.

Making a Transifex Project for Each Repository
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the administrative overhead behind making a Transifex Project for each repository be relieved by creating tooling around user and project management features of the Transifex API?

On the other hand, I'd guess an argument to add is that Transifex translation memory groups are not available at the open source tier of Transifex?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly, but then we have to consider that a mostly non-technical Translation WG has to learn how to use the Transifex API. And then Translators may have to be part of 100+ teams to translate files for each repository, and learn to navigate the hundreds of projects to see their progress.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pshiu for this and some other comments, it would be useful to spell out your use cases (both ones you currently follow and new ones you're proposing - and for the latter, justification to the need)

Copy link
Contributor

Choose a reason for hiding this comment

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

@sarina Sorry for the confusion! My comment was meant to address the rejected alternative of making a Transifex project for each repository. The suggestion was that perhaps tooling could be created to remedy the concerns that caused this alternative be rejected. (For example, perhaps a bot that auto-adds and removes translators from all relevant projects.)

@Carlos-Muniz I thought that even in a consolidated repository scenario, translators still have to navigate one Transifex Resource per original source repository (resulting in the same problem)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pshiu Translators are capable of navigating between Transifex Resources (Translation files) within one Transifex Project, as they do that already with the current Translation Project "edx-platform". File structures are flattened within a transifex project, so they will not have to search through directories to find the resources they are to translate. But having to look between 100+ Transifex projects for 1-2 resources each project, and then expecting Transifex Admin to be able to track the progress of each of the languages for each of the projects becomes a far more labor intensive alternative.

Tooling + a dashboard to track progress would have to be made at the very least. And with all those extra moving parts, we may find ourselves with a pile of spaghetti that would be very difficult to untangle.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Carlos-Muniz After diving into Transifex's UI, here's what I now understand:

That last bullet means (like you said) that to translate or review a string across projects, a translator will either have to:

  1. Click on the editor links in the string search, which loads an editor in a new tab, or
  2. Know which project a string belongs to and switch to that project in the editor's project dropdown menu.

Both are tedious for the translator and a bad user experience. The fact that the Transifex editor does not support editing strings across multiple projects is (at least for me) the winning argument justifying the consolidation of all translation strings into a single Transifex project.

I didn't understand this when I first read the OEP, so I really appreciate your time in enlightening me. Thanks for your help.

Comment on lines +218 to +219
While it won’t directly impact the day-to-day workflow of developers (unless you are
developing or testing with translation files), due to the same reasons that we impact
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to suggest there might be a non-negligible direct impact to a developer's workflow, which is to learn to use the new openedx-atlas tooling and to run (or re-run) it as necessary on pulling each repo a developer works in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Learning how and when to use openedx-atlas may be a non-negligible learning task, but then again so is learning any new package. In the future, it can be added to Makefiles to minimize the learning load just as i18n-tools commands were.

Copy link
Contributor

Choose a reason for hiding this comment

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

I get the impression that for most dev workflows the atlas usage will be pretty straightforward. Any friction can also be mitigated by ensuring repos include a section in the readme describing any simple atlas commands a dev may need to run. Furthermore, atlas use can be integrated into any scripts that currently depend on translation files existing within the repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

@brian-smith-tcril, I really like the README addition idea and integrating openedx-atlas into any scripts we currently use within our repos.

I definitely agree the impact will be minimal; it's clear openedx-atlas was designed with that in mind. But minimal impact does exist, and I think developer education is a great way to address that impact.

Comment on lines +131 to +133
Repositories that generate translation files will have their translation files generated
and committed via a pull request to the openedx-translation repository via a GitHub
workflow. Once the translation files from edx-platform and other repositories are moved
Copy link
Contributor

Choose a reason for hiding this comment

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

What will this GitHub workflow be called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you! Would it be worth adding that link into the OEP?

Copy link
Member

Choose a reason for hiding this comment

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

@pshiu from OEP-1:

OEPs are not used to dictate small decisions made in every day feature work. See OEP-19 for more detail.

I think the workflow name is a small decision. I wouldn't want to lock Carlos et al into that specific workflow name if it's not critical to this OEP's deisgn. Do you see any harm in leaving it out?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not at all! Thought it might be a nice add if it was already decided for reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since OEP-58's next target status is "Provisional", once it is back "Under Review" with the target status of "Accepted", we can update the details of OEP-58 to reflect the specifics of implementation.

Comment on lines +67 to +70
* edx-transifex-bot is a black box for most of the community: the code for the
edx-transifex-bot is in the `ecommerce-scripts`_ repository but it is impossible for
most of the community to observe the work it is doing, or whether it is doing it
correctly. In addition, there is no documentation for these important scripts.
Copy link
Contributor

@pshiu pshiu Nov 15, 2022

Choose a reason for hiding this comment

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

Where are the errors of the Transifex GitHub App shown? I understand that many of the errors will be caught on extraction by the GitHub workflow (and thus will be visible via PR), but I wonder if there are errors that will only be caught at the Transifex GitHub App level.

Assuming such errors exist (they may not), two thought experiments:

  1. What happens if two projects happen to use the same translation key for a string? (Of course, we have a naming convention to prevent that, but say a copy and paste error?)
  2. Will an error caused by one project block translations for all other projects?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a valid concern. Transifex has a very robust notification system, and errors occurring there will notify the Transifex Admins.

For your thought experiments, Transifex projects are treated exclusively. What happens in one, does not affect the others, similar to how GitHub treats repositories. So an error in any Transifex project other than openedx-translations will not affect the translations in Transifex project.

As a side note, translation files were kept in their original file structures to combat the non-unique file name errors we have experienced before, as the entire path is used as the unique translation slug. So as long as files are kept in separate directory structures within the openedx-translations repository, this will not be a problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, great, thanks for the clarification! My sense is that we should be good as long as there is a way for maintainers to know that changes they made caused errors at the Transifex level. It sounds like the Translation WG or Transifex Admins are taking up this role and will be responsible for notifying the relevant maintainer?

I should note that I was being a little confusing in my parent comment because I used "project" to refer to an Open edX Project (a repository) instead of a Transifex Project. I was just trying to make sure an error caused by one repository will not block translations for all other repositories. (I suspect that may have been one of the reasons the original Jenkins jobs that pushed/pulled translations were done on a per-respository basis.)

Comment on lines +131 to +133
Repositories that generate translation files will have their translation files generated
and committed via a pull request to the openedx-translation repository via a GitHub
workflow. Once the translation files from edx-platform and other repositories are moved
Copy link
Contributor

Choose a reason for hiding this comment

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

How will the release branch logistics work exactly? I was worried because the Transifex docs state that "the GitHub integration works on top of a single branch."

Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

At a high level, this proposal to aggregate translations in a single repository sounds fantastic to me. I am interested to see how some of comment threads on the details will shake out, notably around openedx-atlas. I'd also like to see this document follow a more classic OEP format.

Open edX i18n has always been pretty confusing to me, but the proposal here strikes me as refreshingly simple, and I'm excited about all the simplification and deprecation it will lead to.

:local:
:depth: 1

Context
Copy link
Member

Choose a reason for hiding this comment

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

Most OEPs are structured in some variation of:

  • Abstract (very short)
  • Motivation
  • Specification (aka Decision)
  • Impact (aka Consequences)
  • Rejected Alternatives
  • References

I find that this template makes it really easy to digest OEPs, both as a first time reader and as a returning reference-seeker. I find the same thing to be true with PEPs. I think the current draft of this OEP could be mapped into the normal OEP template without any major edits:

  • Decision => Abstract
  • Context, Current State, Rationale => Motivation
  • Decision, Proposed Implementation => Specification
  • Impacts, Locations => Impact

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for this suggestion. I will reorganize the OEP this way at the end of the review period so comments on specific sections are referenced correctly by GitHub.

Comment on lines 53 to 58
To alleviate these issues, we will switch from using the edx-transifex-bot to the
`Transifex GitHub App`_, a stable app provided by Transifex. Benefits of this change
include being easier to maintain and solving a lot of the pain points detailed below. As
part of this proposal, we suggest moving translations into their own repository, to make
using the `Transifex GitHub App`_ more streamlined and straightforward, and in order to
make organizing and using the up to date translations simpler.
Copy link
Member

Choose a reason for hiding this comment

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

Style nit: When writing OEPs/PEPs/RFCs, you don't need to politely suggest things nor write in a conditional voice. The entire OEP is understood as a polite suggestion. So, your language can be more concise & direct. For example, instead of:

As part of this proposal, we suggest moving translations into their own repository, to make
using the Transifex GitHub App_ more streamlined and straightforward, and in order to
make organizing and using the up to date translations simpler.

you can say:

Translations will be moved into their own repository. This will make it easier to use the Transifex GitHub App_, organize translations, and use translations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes sense. I'll update the language to be more concise & direct.

* - Arbiter
- Ned Batchelder <ned@edx.org>
* - Status
- Under Review
Copy link
Member

Choose a reason for hiding this comment

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

@Carlos-Muniz on point 1, @robrap is suggesting that you change the status from:

  • Under Review

to one of these:

  1. Under Review (=> Provisional)
  2. Under Review (=> Accepted)

in order to clarify whether you want this to merge in as a Provisional or Accepted, which is information the reviewers should know. I agree with him.

* - Arbiter
- Ned Batchelder <ned@edx.org>
* - Status
- Under Review
Copy link
Member

Choose a reason for hiding this comment

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

Aside, @robrap : I think this is evidence that the OEP-1 status guidance that we collaborated on could be improved :) OEP-1 presents Under Review as a valid status even though we'd really prefer that folks always include the (=> ...) part.

Carlos Muniz added 2 commits November 22, 2022 11:55
With the conclusion of the review period, conversation threads have been
summarized and added where necessary to provide more clarity. In
addition, the structure of the OEP has been reorganized to be mapped
into the normal OEP template.
@Carlos-Muniz Carlos-Muniz merged commit eded7c0 into openedx:master Dec 1, 2022
@Carlos-Muniz Carlos-Muniz deleted the Carlos-Muniz/translations-management branch December 1, 2022 15:37
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.