From 504f862246cdf6efb40a3b554d0ec861e2aa54a1 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Sat, 4 Nov 2023 17:46:34 +0300 Subject: [PATCH] fixup! feat: mark invalid entries as fuzzy --- .../workflows/automerge-transifex-app-prs.yml | 21 ++----- .github/workflows/ci.yml | 62 ------------------- .github/workflows/python-tests.yml | 31 ++++++++++ .../workflows/validate-translation-files.yml | 56 +++++++++++++++++ docs/decisions/0002-mark-fuzzy-entries.rst | 29 ++++++++- 5 files changed, 119 insertions(+), 80 deletions(-) create mode 100644 .github/workflows/python-tests.yml create mode 100644 .github/workflows/validate-translation-files.yml diff --git a/.github/workflows/automerge-transifex-app-prs.yml b/.github/workflows/automerge-transifex-app-prs.yml index f368dd54465..5df24a43cb5 100644 --- a/.github/workflows/automerge-transifex-app-prs.yml +++ b/.github/workflows/automerge-transifex-app-prs.yml @@ -15,26 +15,15 @@ jobs: uses: actions/checkout@v3 with: fetch-depth: 0 - - name: merge pull request - uses: nick-fields/retry@v2 - id: mergePR + + - name: Auto-merge pull request env: # secrets can't be used in job conditionals, so we set them to env here TRANSIFEX_APP_ACTOR_NAME: "${{ secrets.TRANSIFEX_APP_ACTOR_NAME }}" TRANSIFEX_APP_ACTOR_ID: "${{ secrets.TRANSIFEX_APP_ACTOR_ID }}" # This token requires Write access to the openedx-translations repo GITHUB_TOKEN: ${{ secrets.EDX_TRANSIFEX_BOT_GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.number }} if: "${{ github.actor == env.TRANSIFEX_APP_ACTOR_NAME && github.actor_id == env.TRANSIFEX_APP_ACTOR_ID }}" - with: - retry_wait_seconds: 60 - max_attempts: 5 - timeout_minutes: 15 - retry_on: error - command: | - # Attempt to merge the PR - gh pr merge ${{ github.head_ref }} --rebase --auto - - # The `fromdate | todate` are used merge to validate that `mergedAt` isn't null - # therefore verifying that the pull request was merged successfully. - gh pr view "$PR_NUMBER" --json mergedAt --jq '.mergedAt | fromdate | todate' + run: | + # Add the pull request to the merge queue with --rebase commit strategy + gh pr merge ${{ github.head_ref }} --rebase --auto diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e986ad46646..bdf415a6a7f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,19 +17,6 @@ jobs: - name: clone openedx/openedx-translations uses: actions/checkout@v3 - - name: Install gettext - run: sudo apt install -y gettext - - - name: setup python - uses: actions/setup-python@v4 - with: - python-version: '3.8' - - - name: Run python tests - run: | - make test_requirements - make test - - name: Allow editing translation files for bot pull requests env: # secrets can't be used in job conditionals, so we set them to env here @@ -56,42 +43,6 @@ jobs: exit $has_errors - - name: Commit fixes to git - id: commit_fixes - if: ${{ github.event.repository.full_name == github.event.pull_request.head.repo.full_name }} - run: | - # Commit if there are changes to translation files - if ! git diff --no-ext-diff --quiet --exit-code; then - # Set the git user to the bot user to enable commit - git config --global user.email "translations-bot@openedx.org" - git config --global user.name "edx-transifex-bot" - - # Switch from the merge commit to the pull request branch to enable push - git checkout "${{ github.head_ref }}" - - git add translations/ - git commit --message "fix: mark invalid entries as fuzzy" translations/ - git push - - echo "FUZZY_FIX_COMMIT_SHA=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" - else - echo "No changes to commit" - fi - - - name: Allow merging the fuzzy fix commit - uses: actions/github-script@v6 - if: ${{ steps.commit_fixes.outputs.FUZZY_FIX_COMMIT_SHA }} - with: - script: | - await github.rest.repos.createCommitStatus({ - context: 'tests', - description: 'ci.yml: Forced success status.', - owner: context.repo.owner, - repo: context.repo.repo, - sha: '${{ steps.commit_fixes.outputs.FUZZY_FIX_COMMIT_SHA }}', - state: 'success', - }) - - name: Post translation validation results as a pull request comment # Due to GitHub Actions security reasons posting a comment isn't possible on fork pull requests. # This shouldn't be an issue, because bots writes directly to this repository. @@ -115,16 +66,3 @@ jobs: ``` This comment has been posted by the `validate-translation-files.yml` GitHub Actions workflow. - - - name: Auto-merge pull request - env: - # secrets can't be used in job conditionals, so we set them to env here - TRANSIFEX_APP_ACTOR_NAME: "${{ secrets.TRANSIFEX_APP_ACTOR_NAME }}" - TRANSIFEX_APP_ACTOR_ID: "${{ secrets.TRANSIFEX_APP_ACTOR_ID }}" - # This token requires Write access to the openedx-translations repo - GITHUB_TOKEN: ${{ secrets.EDX_TRANSIFEX_BOT_GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.number }} - if: "${{ github.actor == env.TRANSIFEX_APP_ACTOR_NAME && github.actor_id == env.TRANSIFEX_APP_ACTOR_ID }}" - run: | - # Add the pull request to the merge queue with --rebase commit strategy - gh pr merge ${{ github.head_ref }} --rebase --auto diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml new file mode 100644 index 00000000000..16e2d443f4c --- /dev/null +++ b/.github/workflows/python-tests.yml @@ -0,0 +1,31 @@ +# run make test for python + +name: Python Tests + +on: + - pull_request + +jobs: + python-translations: + runs-on: ubuntu-latest + + steps: + # Clones the openedx-translations repo + - name: clone openedx/openedx-translations + uses: actions/checkout@v3 + + - name: Install gettext + run: | + sudo apt install -y gettext + + # Sets up Python + - name: setup python + uses: actions/setup-python@v4 + with: + python-version: '3.8' + + # Run the script + - name: run python tests + run: | + make test_requirements + make test diff --git a/.github/workflows/validate-translation-files.yml b/.github/workflows/validate-translation-files.yml new file mode 100644 index 00000000000..79eaa78ab42 --- /dev/null +++ b/.github/workflows/validate-translation-files.yml @@ -0,0 +1,56 @@ +# Validate the po files to ensure translation files are compilable. + +name: Validate translation PO files + +on: + pull_request: + +jobs: + validate-po-files: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + steps: + # Clones the openedx-translations repo + - name: clone openedx/openedx-translations + uses: actions/checkout@v3 + + - name: Install gettext + run: | + sudo apt install -y gettext + + - name: Validate translation files + id: validate_translation_files + run: | + has_validation_errors=0 + python scripts/validate_translation_files.py 2>validation_errors.txt || has_validation_errors=1 + cat validation_errors.txt + + { + echo 'VALIDATION_ERRORS<> "$GITHUB_OUTPUT" + + exit $has_validation_errors + + - name: Post translation validation results as a comment + # Due to GitHub Actions security reasons posting a comment isn't possible on fork pull requests. + # This shouldn't be an issue, because bots writes directly to this repository. + if: ${{ always() && !github.event.pull_request.head.repo.fork }} + uses: mshick/add-pr-comment@7c0890544fb33b0bdd2e59467fbacb62e028a096 + with: + message: | + :white_check_mark: All translation files are valid. + + This comment has been posted by the `validate-translation-files.yml` GitHub Actions workflow. + + message-failure: | + :warning: There are errors in the translation files: + + ``` + ${{ steps.validate_translation_files.outputs.VALIDATION_ERRORS }} + ``` + + This comment has been posted by the `validate-translation-files.yml` GitHub Actions workflow. diff --git a/docs/decisions/0002-mark-fuzzy-entries.rst b/docs/decisions/0002-mark-fuzzy-entries.rst index b9853b0380a..0e0bd6219ae 100644 --- a/docs/decisions/0002-mark-fuzzy-entries.rst +++ b/docs/decisions/0002-mark-fuzzy-entries.rst @@ -1,6 +1,11 @@ Mark invalid entries in po files as fuzzy ######################################### +Status +****** + +Rejected. + Context ******* As of the `OEP-58`_, the Transifex GitHub App is used to sync Translations. @@ -14,9 +19,27 @@ Problem There are times when the translations being synchronized fail ``msgfmt`` validation. This prevents the pull requests from being merged. +Proposed Design Decision [Rejected] +*********************************** + +.. note:: + This design decision was rejected because there were multiple issues that + had no feasible solution within the original scope of the `OEP-58`_ + proposal. These issues are listed below: + + - The workflow script edits the translations and effectively hide the + errors, which can be confusing for the translators. Therefore, + a good notification should be + sent to translators to avoid silencing errors. + - Writing to the pull request will not trigger the GitHub Actions workflow, + therefore the pull request cannot be merged unless the commit status + is overridden manually via `GitHub "create a commit status" API`_. + - The solution is ``.po`` file specific. And there's no similar solution for + ``.json``, ``.yaml``, iOS, Android or other translations files. + + Please refer to the original pull request which contains the rejected + implementation: `mark invalid entries as fuzzy | FC-0012`_. -Design Decision -*************** A GitHub Actions workflow will be implemented to mark invalid entries in synchronized ``.po`` files as ``fuzzy``. This will update pull requests @@ -94,3 +117,5 @@ Rejected Alternative: Keep pull requests open .. _OEP-58: https://open-edx-proposals.readthedocs.io/en/latest/architectural-decisions/oep-0058-arch-translations-management.html .. _LexiQA: https://help.transifex.com/en/articles/6219179-lexiqa .. _Transifex GitHub App: https://github.com/apps/transifex-integration +.. _GitHub "create a commit status" API: https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#create-a-commit-status +.. _mark invalid entries as fuzzy | FC-0012: https://github.com/openedx/openedx-translations/pull/1944