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.

Sign up for GitHub

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

Fix 17050actions warning message #17098

Closed
wants to merge 3 commits into from
Closed

Fix 17050actions warning message #17098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6fbbc10
add ability to translate the user guide on Crowdin (#17050)
michaelDCurran Sep 2, 2024
c680a94
uses actions/checkout@v4 and actions/setup-python@v5 fix
dpy013 Sep 2, 2024
dffa98f
Merge branch 'nvaccess:master' into fix-actions
dpy013 Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions .github/workflows/regenerate_english_userDocs_translation_source.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
name: Rebuild English User Documentation for Translation

on:
push:
branches:
- beta
paths:
- 'user_docs/en/*.md'

jobs:
rebuild-translation-source:
runs-on: windows-latest

steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.11'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install lxml requests

- name: update xliff files
shell: pwsh
run: |
# for any English markdown files changed within the commits of this push,
# update the corresponding xliff file (if one exists) to reflect the current markdown file,
# keeping existing translation IDs in tact.
$ErrorActionPreference = 'Stop'
$changedFiles = git diff --name-only ${{github.event.before}}.. -- user_docs/en/*.md
foreach ($file in $changedFiles) {
Write-Host "$file has changed"
$baseName = [System.IO.Path]::GetFileNameWithoutExtension($file)
$xliff = "user_docs/en/$baseName.xliff"
$tempXliff = "user_docs/en/$baseName.xliff.temp"
$markdown = $file
if (Test-Path $xliff) {
Write-Host "Updating $xliff with changes from $markdown"
python user_docs/markdownTranslate.py updateXliff -x $xliff -m $file -o $tempXliff
Write-Host "Renaming $tempXliff to $xliff"
move-item -Path $tempXliff -Destination $xliff -Force
} else {
Write-Host "Ignoring $markdown as it does not have a corresponding xliff file"
}
}
if: success()
Comment on lines +30 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

XLIFF update process is well-implemented, but could be optimized.

The script efficiently updates XLIFF files for changed markdown files. However, there's room for improvement:

  1. Consider using a more efficient git command to get changed files.
  2. The error handling could be more robust.

Consider the following optimizations:

  1. Replace the git command with:
-$changedFiles = git diff --name-only ${{github.event.before}}.. -- user_docs/en/*.md
+$changedFiles = git diff --name-only ${{github.event.before}} ${{github.sha}} -- user_docs/en/*.md
  1. Add error handling for the Python script execution:
 python user_docs/markdownTranslate.py updateXliff -x $xliff -m $file -o $tempXliff
+if ($LASTEXITCODE -ne 0) {
+    Write-Error "Failed to update XLIFF file: $xliff"
+    exit 1
+}


- name: Commit and Push changes
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
$ErrorActionPreference = 'Stop'
git config --local user.name "GitHub Actions"
git config --local user.email "[email protected]"
git remote set-url origin https://x-access-token:${GITHUB_TOKEN}@github.com/${{ github.repository }}.git
$filesChanged = git diff --name-only -- *.xliff
if ($filesChanged) {
Write-Host "xliff files were changed. Committing and pushing changes."
foreach ($file in $filesChanged) {
git add $file
git commit -m "Update $file"
}
git push origin HEAD
} else {
Write-Host "No xliff files were changed. Skipping commit and push."
}
if: success()
Comment on lines +55 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

Commit and push process is well-implemented, but could be more efficient.

The process correctly commits and pushes changes for modified XLIFF files. However, there's an opportunity to optimize the git operations.

Consider the following optimizations:

  1. Use a single commit for all changed files:
-foreach ($file in $filesChanged) {
-  git add $file
-  git commit -m "Update $file"
-}
+git add *.xliff
+git commit -m "Update XLIFF files"
  1. Use git diff --quiet for a more efficient check:
-$filesChanged = git diff --name-only -- *.xliff
-if ($filesChanged) {
+if (-not (git diff --quiet -- *.xliff)) {


- name: Crowdin upload
# This step must only be run after successfully pushing changes to the repository.
# Otherwise if the push fails, subsequent runs may cause new translation IDs to be created,
# which will cause needless retranslation of existing strings.
env:
crowdinProjectID: ${{ vars.CROWDIN_PROJECT_ID }}
crowdinAuthToken: ${{ secrets.CROWDIN_AUTH_TOKEN }}
run: |
# Check if we changed userGuide.xliff in this action.
# If we did, upload it to Crowdin.
$ErrorActionPreference = 'Stop'
$changed = git diff --name-only ${{GITHUB.SHA}}.. -- user_docs/en/userGuide.xliff
if ($changed) {
Write-Host "Uploading userGuide.xliff to Crowdin"
# 18 is the file ID for userGuide.xliff in Crowdin.
python appVeyor/crowdinSync.py uploadSourceFile 18 user_docs/en/userguide.xliff
} else {
Write-Host "Not uploading userGuide.xliff to Crowdin as it has not changed"
}
Comment on lines +76 to +94
Copy link
Contributor

Choose a reason for hiding this comment

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

Crowdin upload process is correctly implemented, but could be improved.

The process checks for changes in userGuide.xliff and uploads it to Crowdin if changed. However, there are a few points to consider:

  1. The comment mentions a file ID (18) for userGuide.xliff in Crowdin. This might be better stored as a variable or environment variable for easier maintenance.
  2. The git command to check for changes could be more efficient.
  3. Error handling for the Python script execution could be improved.

Consider the following improvements:

  1. Store the Crowdin file ID as an environment variable:
 env:
   crowdinProjectID: ${{ vars.CROWDIN_PROJECT_ID }}
   crowdinAuthToken: ${{ secrets.CROWDIN_AUTH_TOKEN }}
+  crowdinUserGuideFileID: 18
  1. Use a more efficient git command:
-$changed = git diff --name-only ${{GITHUB.SHA}}.. -- user_docs/en/userGuide.xliff
+$changed = git diff --quiet ${{GITHUB.SHA}} HEAD -- user_docs/en/userGuide.xliff
+if ($LASTEXITCODE -eq 1) {
  1. Add error handling for the Python script:
 python appVeyor/crowdinSync.py uploadSourceFile 18 user_docs/en/userguide.xliff
+if ($LASTEXITCODE -ne 0) {
+    Write-Error "Failed to upload userGuide.xliff to Crowdin"
+    exit 1
+}

11 changes: 11 additions & 0 deletions sconstruct
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,17 @@ for po in env.Glob(sourceDir.path + "/locale/*/lc_messages/*.po"):
styles = os.path.join(userDocsDir.path, "styles.css")
numberedHeadingsStyle = os.path.join(userDocsDir.path, "numberedHeadings.css")

# Create Non-english md files in user_docs from localized xliff and English skel files
for xliffFile in env.Glob(os.path.join(userDocsDir.path, "*", "*.xliff")):
lang = os.path.basename(xliffFile.path).split(".")[0]
if lang == "en":
continue
mdFile = os.path.splitext(xliffFile.path)[0] + ".md"
env.Command(
mdFile,
xliffFile,
[f'@{sys.executable} user_docs/markdownTranslate.py generateMarkdown -x "{xliffFile}" -o "{mdFile}"'],
)
# Allow all markdown files to be converted to html in user_docs
for mdFile in env.Glob(os.path.join(userDocsDir.path, "*", "*.md")):
htmlFile = env.md2html(mdFile)
Expand Down
5,022 changes: 5,022 additions & 0 deletions tests/markdownTranslate/en_2024.2_userGuide.md
View file
Open in desktop

Large diffs are not rendered by default.

Loading