fixXliffRegeneration#19054
Conversation
There was a problem hiding this comment.
Pull Request Overview
Fixes the broken GitHub action for regenerating English user documentation xliff files for translation by replacing SSH key authentication with the standard GitHub token authentication.
- Removes SSH key setup and configuration code
- Updates checkout action to v4 and adds permissions for write access
- Simplifies push command to use GitHub token authentication
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
This has been tested by including the pr branch in the list of branches the action should run on. Then making a change to the english user guide, and ensuring the action can commit / push to the current branch. |
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| persist-credentials: true # sets origin with GITHUB_TOKEN |
There was a problem hiding this comment.
using the default github token means that any pushes to beta will not trigger a CI build. I think we should be pushing using nvaccessAuto credentials so beta builds will go out from pushes
There was a problem hiding this comment.
Technically, no CI build is required for the push made by this action, as all it does is update the English userGuide.xliff file, which is used for future translations. This is then sent to Crowdin. The only reason it is committed / pushed here is for caching translation source string IDs so that Crowdin can tell new from existing strings. Note that even the English HTML user guide is generated from the original english md file, not the xliff.
If we would really rather go with NVAccessAuto, then I'll close this PR, and create a new SSH key, add it to the nvAccessAuto account and replace the secret in this repository. But if we can get away with no CI for this push, then what this PR does is much more standard and cleaner than having to worry about expiring tokens. (#19054 (comment))
I think we still need CI builds, otherwise we can't create tagged builds. We require a successful CI build on the latest commit from beta to push a tag for beta
There was a problem hiding this comment.
Additionally, we can create a token which doesn't expire.
|
Technically, no CI build is required for the push made by this action, as all it does is update the English userGuide.xliff file, which is used for future translations. This is then sent to Crowdin. The only reason it is committed / pushed here is for caching translation source string IDs so that Crowdin can tell new from existing strings. Note that even the English HTML user guide is generated from the original english md file, not the xliff. |
|
Closing this pr as it will not trigger CI builds, which would be required if this happened to be the tip of the branch that was tagged for a release. |
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v3 | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
This action already has V5 version
| uses: actions/checkout@v4 | |
| uses: actions/checkout@v5 |
### Link to issue number: <!-- Use Closes/Fixes/Resolves #xxx to link this PR to the issue it is responding to. --> Fixes #19023 Replaces pr #19054 ### Summary of the issue: The GitHub action that regenerates updated xliff files for translation, from English user documentation is currently broken, as the ssh credentials uses an out-of-date key. ### Description of user facing changes: ### Description of developer facing changes: ### Description of development approach: The GitHub action now uses a GitHub Personal Access Token (PAT) that has permission to push to the repository, and will still trigger future actions. ### Testing strategy: * [x] Run and ensure it successfully pushes to the pr branch, by temporarily listing the pr branch for the action. * [ ] Once merged to beta, ensure the action correctly runs. ### Known issues with pull request: None known. ### Code Review Checklist: <!-- This checklist is a reminder of things commonly forgotten in a new PR. Authors, please do a self-review of this pull-request. Check items to confirm you have thought about the relevance of the item. Where items are missing (eg unit / system tests), please explain in the PR. To check an item `- [ ]` becomes `- [x]`, note spacing. You can also check the checkboxes after the PR is created. A detailed explanation of this checklist is available here: https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/githubPullRequestTemplateExplanationAndExamples.md#code-review-checklist --> - [x] Documentation: - Change log entry - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [x] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English - [x] API is compatible with existing add-ons. - [x] Security precautions taken. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: GitHub Actions <actions@github.com>
Link to issue number:
Fixes #19023
Summary of the issue:
The GitHub action that regenerates updated xliff files for translation, from English user documentation is currently broken, as the ssh credentials uses an out-of-date key.
Description of user facing changes:
Description of developer facing changes:
Description of development approach:
The GitHub action now uses the standard auto-generated GitHub token provided to the action which has permission to push to the current repository.
Testing strategy:
Known issues with pull request:
Code Review Checklist: