docs: add the Python Repo section into OEP-58 docs #366

OmarIthawi · 2023-08-08T12:25:04Z

document the new translation workflow for Python Repo.

I'll be filling the other sections in upcoming PRs.

For now the controversial part is whether we should be recommending i18n_tool extract or use django-admin.py makemessages. My memory isn't helping me to determine which does what, so I'll be asking for help from @shadinaif and of course @sarina who have used both a ton.

Preview the file here: https://github.com/Zeit-Labs/docs.openedx.org/blob/mfe-atals-docs/source/developers/how-tos/enable-translations-new-repo.rst

This pull request is open against a parent one that'll be merged later:

docs: Add translations how-to articles #363

This contribution is part of the FC-0012 project which is sparked by the Translation Infrastructure update OEP-58.

openedx-webhooks · 2023-08-08T12:25:08Z

Thanks for the pull request, @OmarIthawi! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

brian-smith-tcril

Thank you so much for writing up these docs! I left quite a few comments but overall this is wonderful! Looking forward to seeing these come together!

brian-smith-tcril · 2023-08-21T14:10:05Z

source/developers/how-tos/enable-translations-new-repo.rst

+Enabling translations on a new repo is mostly starting from an up-to-date cookiecutter
+and the rest is ensuring that the repo is configured correctly.


I think linking to the cookiecutter and configuration docs might be helpful, maybe something like:

"The easiest way to enable translations on a new repository is to use an up-to-date cookiecutter, and ensure the repository is configured correctly"

brian-smith-tcril · 2023-08-21T14:11:28Z

source/developers/how-tos/enable-translations-new-repo.rst

+Enabling translations on a new repo is mostly starting from an up-to-date cookiecutter
+and the rest is ensuring that the repo is configured correctly.
+
+However, this guide will assume that you're starting from scratch.


I'm not sure what you mean by "starting from scratch" here, it seems both the python and react repo instructions mention cookiecutters/templates.

brian-smith-tcril · 2023-08-21T14:14:54Z

source/developers/how-tos/enable-translations-new-repo.rst

+The `OEP-58: Translations Management <https://docs.openedx.org/projects/openedx-proposals/en/latest/architectural-decisions/oep-0058-arch-translations-management.html>`_
+proposal introduced new workflows and tooling to the Open edX translation management:
+
+ - Created the new `openedx-translations project <https://app.transifex.com/open-edx/openedx-translations/dashboard/>`_
+   in Transifex.
+ - Created the new `openedx-translations repo <https://github.com/openedx/openedx-translations>`_
+   in GitHub.
+ - Setup two-way links between the Transifex project and the GitHub repo via the
+   `Transifex GitHub app <https://github.com/apps/transifex-integration>`_.
+ - Added new the `atlas <https://github.com/openedx/openedx-atlas>`_ command line tool to pull translations which will be used in the repo Makefiles, build and deploy processes to fetch translations from the
+   `openedx-translations repo <https://github.com/openedx/openedx-translations>`_.
+
+The full background and rational for the new workflows and tooling can be found in the
+`OEP-58: Translations Management <https://docs.openedx.org/projects/openedx-proposals/en/latest/architectural-decisions/oep-0058-arch-translations-management.html>`_
+docs.


I think it might make sense to have this be at the top of this document. It seems everything above this is also covered in the individual repo type sections, so that may not be needed.

source/developers/how-tos/enable-translations-new-repo.rst

brian-smith-tcril · 2023-08-21T14:19:47Z

source/developers/how-tos/enable-translations-new-repo.rst

+   `openedx-translations repo <https://github.com/openedx/openedx-translations>`_.
+
+The full background and rational for the new workflows and tooling can be found in the
+`OEP-58: Translations Management <https://docs.openedx.org/projects/openedx-proposals/en/latest/architectural-decisions/oep-0058-arch-translations-management.html>`_


nitpick: we tend to not use inline links in the middle of paragraphs like this. This is mostly for readability when editing the rst files, it's nice to just see

The full background and rationale for the new workflows and tooling can be found in the `OEP-58: Translations Management`_ docs.

source/developers/how-tos/enable-translations-new-repo.rst

brian-smith-tcril

Overall this is looking great! Left a few comments with suggestions to make some parts a little less wordy, and a couple with questions about the order of instructions (what should be done pre/post PR)

source/developers/how-tos/enable-translations-new-repo.rst

brian-smith-tcril · 2023-09-05T15:32:34Z

source/developers/how-tos/enable-translations-new-repo.rst

+7. Install the XBlock or plugin in a devstack and run the ``OPENEDX_ATLAS_PULL=true make pull_translations`` in
+   the edx-platform repo to preview the translations in devstack.
+
+   This step is somewhat oversimplified and assumes you're familiar with the Open edX development workflow whether
+   you're using `Tutor <https://docs.tutor.overhang.io/>`_ or the
+   `Open edX devstack <https://github.com/openedx/devstack/>`_.


does it make sense to move this step above the PR step? it seems like testing locally is something that should happen before PRing, but the listed command won't work pre-PR

brian-smith-tcril · 2023-09-05T15:32:50Z

source/developers/how-tos/enable-translations-new-repo.rst

+7. Run the ``OPENEDX_ATLAS_PULL=true make pull_translations`` to verify that it pulls the translations from the
+   `openedx-translations repo`_ into ``conf/locale`` directory. Depending on the repo
+   ``make static``/``make static.dev`` is usually needed to generate the JavaScript translation files.


does it make sense to move this step above the PR step? it seems like testing locally is something that should happen before PRing, but the listed command won't work pre-PR

source/developers/how-tos/enable-translations-new-repo.rst

OmarIthawi · 2023-09-05T21:00:31Z

@brian-smith-tcril Thanks a lot for the review and suggestions. It's much better now.

I've tried to address all review comments and I've added inline TODOs where I intend to make further PRs.

This PR is ready for another review/merge.

brian-smith-tcril

This looks wonderful! I left a couple comments with tiny nitpicks but this is 99% good to go!

I also left one comment in there with a suggestion that might make it so people don't need to scroll up and down as much, but I wouldn't consider that a blocker for merging this.

source/developers/how-tos/enable-translations-new-repo.rst

brian-smith-tcril · 2023-09-06T16:58:30Z

source/developers/how-tos/enable-translations-new-repo.rst

+#. Run ``OPENEDX_ATLAS_PULL=true make pull_translations`` to verify translations are pulled from the
+   `openedx-translations repo`_ into the ``conf/locale`` directory. To generate JavaScript translation files you will
+   likely also need to run ``make static``/``make static.dev``.


Variations of this exist in all 3 types of repo instructions. Maybe it makes sense to move it down into the debugging section instead of having people scroll down and then back up?

Good point @brian-smith-tcril, but I'm reluctant to merge them.

The three variations would may end up in a large item with 3-bullet points which is slightly more complicated than scrolling up and down.

I'd like to keep it as-is and we can iterate later.

source/developers/how-tos/enable-translations-new-repo.rst

feanil

I spotted a couple of structural issues in my review that I think should be changed, but I did not thoroughly review the content.

feanil · 2023-09-07T18:38:47Z

source/developers/how-tos/enable-translations-new-repo.rst

+============
+Introduction
+============
+
+
+The `OEP-58 - Translations Management`_ proposal introduced new workflows and tooling to the Open edX translation
+management:
+
+- Created the new `openedx-translations project`_ in Transifex.
+- Created the new `openedx-translations repo`_ in GitHub.
+- Setup two-way links between the Transifex project and the GitHub repo via the
+  `Transifex GitHub app <https://github.com/apps/transifex-integration>`_.
+- Added new the `atlas <https://github.com/openedx/openedx-atlas>`_ command line tool to pull translations which will
+  be used in the repo Makefiles, build and deploy processes to fetch translations from the
+  `openedx-translations repo`_.
+
+After configuring the repo as shown in the sections below, the translations workflow will automatically:
+
+- Add the extracted source (English) translation files from the repository you added to the
+  `openedx-translations repo`_ in GitHub.
+- Create a new Transifex resource for your repo in the `openedx-translations project`_ in Transifex.
+- Add the translations (Arabic, French, etc) to the GitHub `openedx-translations repo`_ when translation entries are
+  updated by translators.
+
+The full background and rationale for the new workflows and tooling can be found in the
+`OEP-58 - Translations Management`_ docs.
+


This section is useful context but I think belongs in a concepts doc probably one titled Translation Technology and Process. Then I think you could add a link to that in a ..seealso section of this document.

I've removed this section and focused on the How-to part.

This section is a summary of the OEP-58 document. So if we're aiming for brevity, a link should suffice.

I have the habit of summerization from the time that I used to write some academic research.

feanil · 2023-09-07T18:39:05Z

source/developers/how-tos/enable-translations-new-repo.rst

+The full background and rationale for the new workflows and tooling can be found in the
+`OEP-58 - Translations Management`_ docs.
+
+The next sections cover the steps to enable translations on a new repo depending its type:


I don't think you need this line.

feanil · 2023-09-07T18:40:09Z

source/developers/how-tos/enable-translations-new-repo.rst


-TODO: How to enable translations infrastructure on a Python repo
+==========


Please use the heading structure describe in the Documentation quick-reference instead of using double headings. https://docs.openedx.org/en/latest/documentors/references/quick_reference_rst.html#headings

feanil · 2023-09-07T18:41:20Z

source/developers/how-tos/enable-translations-new-repo.rst

+#. For existing repos which don't have the ``make extract_translations`` command, it can be copied from the
+   `edx-cookiecutters`_ Makefile in the corresponding template e.g. ``cookiecutter-xblock`` Makefile for XBlocks.
+
+   **Note:** Some repositories use ``django-manage makemessages``. The recommendation is to use ``i18n_tool extract``


Use a ..note admonition instead: https://www.sphinx-doc.org/en/master/usage/restructuredtext/directives.html#directive-note

feanil · 2023-09-07T18:42:58Z

source/developers/how-tos/enable-translations-new-repo.rst

+#. If you are creating a new repository, use the `cookiecutter-django-ida`_
+   template.
+
+**TODO:** How to enable translations infrastructure on a Django microservice


Is this todo still relevant? Seems like you answer it below.

Yes, it needs expansion and I'll follow up in other PRs.

OmarIthawi · 2023-09-09T13:33:03Z

@feanil thanks for the comments. The PR is ready for another review.

feanil

One last change and then it looks good to me from a syntax and structure perspective.

feanil · 2023-09-12T14:48:29Z

source/developers/how-tos/enable-translations-new-repo.rst


-TODO: How to enable translations infrastructure on a Python repo
+Quickstart
+*********


Suggested change

*********

**********

Needs to match the length.

Done @feanil!

document the new translation workflow This contribution is part of the [FC-0012 project](https://openedx.atlassian.net/l/cp/XGS0iCcQ) which is sparked by the [Translation Infrastructure update OEP-58](https://open-edx-proposals.readthedocs.io/en/latest/architectural-decisions/oep-0058-arch-translations-management.html#specification).

OmarIthawi · 2023-09-13T09:56:12Z

@brian-smith-tcril would you mind checking if it's ready to merge?

So I can start merging adding the other sections.

feanil · 2023-09-13T14:52:41Z

@brian-smith-tcril feel free to merge yourself once you think this is good.

openedx-webhooks · 2023-09-18T15:48:21Z

@OmarIthawi 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 8, 2023

OmarIthawi requested a review from sarina August 8, 2023 12:25

OmarIthawi requested a review from brian-smith-tcril August 15, 2023 15:19

brian-smith-tcril reviewed Aug 21, 2023

View reviewed changes

OmarIthawi force-pushed the mfe-atals-docs branch 6 times, most recently from 2f51afc to 110dd4b Compare September 3, 2023 12:21

OmarIthawi requested a review from brian-smith-tcril September 3, 2023 12:23

OmarIthawi force-pushed the mfe-atals-docs branch 8 times, most recently from bfbd690 to 619d20c Compare September 4, 2023 04:54

brian-smith-tcril reviewed Sep 5, 2023

View reviewed changes

OmarIthawi commented Sep 5, 2023

View reviewed changes

source/developers/how-tos/enable-translations-new-repo.rst Outdated Show resolved Hide resolved

OmarIthawi force-pushed the mfe-atals-docs branch from 035b70d to 42b5608 Compare September 5, 2023 16:13

OmarIthawi requested a review from brian-smith-tcril September 5, 2023 16:14

OmarIthawi force-pushed the mfe-atals-docs branch from cf7c7d9 to 612d8b9 Compare September 5, 2023 20:59

brian-smith-tcril approved these changes Sep 6, 2023

View reviewed changes

OmarIthawi force-pushed the mfe-atals-docs branch from a4aa32e to 4c884b6 Compare September 6, 2023 18:46

feanil self-requested a review September 7, 2023 17:35

feanil reviewed Sep 7, 2023

View reviewed changes

OmarIthawi requested a review from feanil September 9, 2023 13:30

OmarIthawi force-pushed the mfe-atals-docs branch from 4c884b6 to 71ebcc4 Compare September 9, 2023 13:38

feanil reviewed Sep 12, 2023

View reviewed changes

OmarIthawi force-pushed the mfe-atals-docs branch from 71ebcc4 to 2aba751 Compare September 13, 2023 09:52

brian-smith-tcril merged commit 5e5387a into openedx:oep-58-howto Sep 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: add the Python Repo section into OEP-58 docs #366

docs: add the Python Repo section into OEP-58 docs #366

OmarIthawi commented Aug 8, 2023 •

edited

Loading

openedx-webhooks commented Aug 8, 2023 •

edited

Loading

brian-smith-tcril left a comment

brian-smith-tcril Aug 21, 2023

brian-smith-tcril Aug 21, 2023

brian-smith-tcril Aug 21, 2023

brian-smith-tcril Aug 21, 2023

brian-smith-tcril left a comment

brian-smith-tcril Sep 5, 2023

brian-smith-tcril Sep 5, 2023

OmarIthawi commented Sep 5, 2023 •

edited

Loading

brian-smith-tcril left a comment

brian-smith-tcril Sep 6, 2023

OmarIthawi Sep 6, 2023

feanil left a comment

feanil Sep 7, 2023

OmarIthawi Sep 9, 2023

feanil Sep 7, 2023

feanil Sep 7, 2023

feanil Sep 7, 2023

feanil Sep 7, 2023

OmarIthawi Sep 9, 2023

OmarIthawi commented Sep 9, 2023

feanil left a comment

feanil Sep 12, 2023

OmarIthawi Sep 13, 2023

OmarIthawi commented Sep 13, 2023 •

edited

Loading

feanil commented Sep 13, 2023

openedx-webhooks commented Sep 18, 2023

		Enabling translations on a new repo is mostly starting from an up-to-date cookiecutter
		and the rest is ensuring that the repo is configured correctly.


		TODO: How to enable translations infrastructure on a Python repo
		==========

docs: add the Python Repo section into OEP-58 docs #366

docs: add the Python Repo section into OEP-58 docs #366

Conversation

OmarIthawi commented Aug 8, 2023 • edited Loading

openedx-webhooks commented Aug 8, 2023 • edited Loading

brian-smith-tcril left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brian-smith-tcril left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmarIthawi commented Sep 5, 2023 • edited Loading

brian-smith-tcril left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feanil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmarIthawi commented Sep 9, 2023

feanil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmarIthawi commented Sep 13, 2023 • edited Loading

feanil commented Sep 13, 2023

openedx-webhooks commented Sep 18, 2023

OmarIthawi commented Aug 8, 2023 •

edited

Loading

openedx-webhooks commented Aug 8, 2023 •

edited

Loading

OmarIthawi commented Sep 5, 2023 •

edited

Loading

OmarIthawi commented Sep 13, 2023 •

edited

Loading