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

Improve the CI and the translation in readthedocs #38

Merged
merged 35 commits into from
May 30, 2024

Conversation

rffontenelle
Copy link
Collaborator

@rffontenelle rffontenelle commented May 6, 2024

This greatly improves the CI workflow of sphinx-doc-translations, by simplify some statements in CI workflow, removing obsolete PO files and removing obsolete Transifex resources.

  • Notable changes to main.yml:

    • simplifies the original workflow, removing unnecessary statements like git fetch depth, git checkout master, git push actions (in favor of a simple "git push"), etc.¹
    • script updates.sh was split in generate_templates.sh (POT and .tx/config generation) and updates.sh (tx push and tx pull). This was required for the cleanup of POT and Transifex resources.²
    • script locale/lock-translations.py added to find obsolete resources in Transifex, lock for 3 days (time to a manual check) and then remove the resource if it still locked.³
    • Obsolete POT are being removed after lock-translations.py run and before updating translations 4
    • renames job name 'ubuntu' to update
    • Use python version '3' instead of '3.9' to follow Sphinx CI workflow settings
    • only commit when there is a change, avoiding error exit when there is nothing to commit
    • do not push commits when in a pull request to allow testing; commit is made, but isn't really pushed
    • limit write permissions to 'contents', instead of everything as 'write'
  • New workflow file test-translations.yml 5:

    • triggered when *.po is changed on push or pull_request events
    • it builds translated docs and lint PO for all languages and show their errors
    • uses problem-matchers to annotate issues issues (i.e. show in summary and in committed files themselves)
    • this workflow hasn't run in this PR check, because it doesn't exist in master branch yet. See this action run in my fork for an example of run and the syntax errors in the translations.

Footnotes:

¹ Example of issue: In pull requests, changes to updates.sh in the branch would not be used because git checkout master was changing the current branch during the workflow run, effectively making the workflow use updates.sh from master branch.

² locking resources needed POT and .tx/config generated, but not the translations pulled. So splitting seemed to be the best alternative. Other alternative was to set up another GHA job in same file and duplicate lines of statements checkout, dependencies installation, etc.

³ Fixes #40

4 Fixes #42

5 Fixes #41

@rffontenelle
Copy link
Collaborator Author

https://readthedocs.org/api/v2/build/24311043.txt

loading translations [pt-br]... WARNING: reading error: /home/docs/checkouts/readthedocs.org/user_builds/sphinx-pt-br/checkouts/38/locale/pt-br/LC_MESSAGES/sphinx.po, expected only letters, got 'pt-br'

This reverts commit 568a2dd.

This issue is already solved by another commit, hence reverting.
This reverts commit 1647b7b.
@rffontenelle rffontenelle reopened this May 7, 2024
rffontenelle and others added 8 commits May 15, 2024 10:59
This also drops forcing master. On push or schedule,
it will run on master as per event trigger settings.
On pull request, it should run in the branch of the
pull request.
GHA complained ./locale/update.sh: 8: Bad substitution
I don't see a particular reason for this, and it could
prevent other workflows that we could want to be ran.
Centralized build and lint for all languages
The update.sh deletes PO files before pulling new translations. Then we have modified files (updated) and deleted (obsolete, nothing pulled). This commit removes from git the deleted files.
Copy link
Member

@shimizukawa shimizukawa left a comment

Choose a reason for hiding this comment

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

LGTM!

Thank you very much.
I checked the all GHA operation and found no problems.
I think this PR will allow us to close #16 as well.
I also commented one minor point.

As a side note, I ran it with act' to check the behavior in my local environment. the following adjustments were necessary when using act' to check the behavior.

  1. change the matrix in test-translations.yml to language: ["ja"] etc. to have one target language and avoid disk-full.
  2. change apt-get install line to sudo apt-get update && sudo apt-get install -y graphviz.
  3. change make html ... to TZ=UTC make html ... to avoid babel error

locale/lock-translations.py Outdated Show resolved Hide resolved
@shimizukawa shimizukawa mentioned this pull request May 26, 2024
3 tasks
@rffontenelle
Copy link
Collaborator Author

@AA-Turner Would you like to review this? Asking just to know if I wait or merge

Replace [[ with [ for POSIX shell compatibility,
and check for empty variable (e.g. in own local shell)
@rffontenelle rffontenelle merged commit 3210188 into master May 30, 2024
2 checks passed
@rffontenelle rffontenelle deleted the rffontenelle-patch-1 branch May 30, 2024 13:06
@rffontenelle rffontenelle mentioned this pull request Jun 7, 2024
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.

Remove obsolete POT files Implement syntax checking workflow Remove obsolete Tx resources
2 participants