Conversation
There was a problem hiding this comment.
Pull request overview
Adds a Node/TypeScript-based proof-of-concept to generate/update a crowdin.yml config from markdown sources and run a GitHub Actions workflow that uploads sources to Crowdin and triggers a downstream translations workflow.
Changes:
- Introduces a TS script (
src/updateCrowdin.ts) to discover markdown files and generate/updatecrowdin.yml, exporting discovered paths via$GITHUB_OUTPUT. - Adds Node project scaffolding (
package.json,package-lock.json,.node-version) and a simple.gitignore. - Adds a GitHub Actions workflow (
.github/workflows/crowdin.yml) plus a composite setup-node action to run the sync on pushes tomain.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/updateCrowdin.ts |
Scans docs for .md files, generates Crowdin config content, and sets an Actions output with found files. |
package.json |
Adds Node/TS tooling and a script to run the generator. |
package-lock.json |
Locks npm dependencies for the new tooling. |
.node-version |
Pins the Node.js version used by workflows/tooling. |
.gitignore |
Ignores local Node/ENV/IDE artifacts. |
.github/workflows/crowdin.yml |
Workflow to install deps, generate config, run Crowdin action, and trigger downstream workflow dispatch. |
.github/actions/setup-node/action.yaml |
Composite action to set up Node based on .node-version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const mdFiles = findMdFiles(DOCS_DIR); | ||
|
|
||
| if (mdFiles.length === 0) { | ||
| console.warn(` --- No file found in "${DOCS_DIR}". ---`); | ||
| return; | ||
| } | ||
|
|
||
| const crowdinFiles: CrowdinFileEntry[] = mdFiles.map((sourcePath) => { | ||
| const translationPath = sourcePath.replace(`${DOCS_DIR}/`, `${DOCS_DIR}/%locale%/`); | ||
| return { source: sourcePath, translation: translationPath }; | ||
| }); |
There was a problem hiding this comment.
findMdFiles(DOCS_DIR) will also pick up translated markdown files once they exist under docs/.../<locale>/... (as implied by the generated translationPath). On subsequent runs this will cause translated files to be treated as sources and generate incorrect/duplicated entries. Filter the scan to only include source-language files (e.g., skip first-level locale directories under DOCS_DIR or exclude paths matching /${DOCS_DIR}/[a-z]{2}(-[A-Z]{2})?/).
| if (mdFiles.length === 0) { | ||
| console.warn(` --- No file found in "${DOCS_DIR}". ---`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
When no markdown files are found, the function returns before writing found_files to $GITHUB_OUTPUT. The workflow later consumes steps.extract_files.outputs.found_files, so this will be empty/undefined and can break the downstream gh workflow run ... -f files=... call. Consider always setting the output (e.g., to []) or failing the job if zero files is unexpected.
| const { files, base_path, preserve_hierarchy, ...rest } = loadedConfig; | ||
| otherConfigProps = rest; | ||
| } | ||
| } catch (e) { console.error(' --- Error, crowdin.yaml not found ---', e); } |
There was a problem hiding this comment.
The error message says crowdin.yaml not found, but this script reads crowdin.yml (CONFIG_FILE). Also, this catch can be triggered by YAML parse errors, not just a missing file, so the message is misleading. Update the message to reference the correct filename and distinguish read vs parse failures.
| try { | ||
| fs.writeFileSync(CONFIG_FILE, finalOutput, 'utf8'); | ||
| console.log(` --- ${CONFIG_FILE} updated.`); | ||
| } catch (e) { console.error(' --- There was an error saving crowdin.yaml ---', e); } |
There was a problem hiding this comment.
This log/error message refers to crowdin.yaml, but the configured output file is crowdin.yml (CONFIG_FILE). Use the same filename in messages to avoid confusion when debugging workflow failures.
| - name: Trigger download workflow | ||
| env: | ||
| GH_TOKEN: ${{ secrets.PAT_TOKEN }} | ||
| run: | |
There was a problem hiding this comment.
The workflow requires secrets.PAT_TOKEN for the gh workflow run ... step, but the PR description only mentions CROWDIN_PROJECT_ID and CROWDIN_PERSONAL_TOKEN. Either document PAT_TOKEN in the PR description or switch to GITHUB_TOKEN / a GitHub App token if cross-repo workflow dispatch requires it.
| if (fs.existsSync(CONFIG_FILE)) { | ||
| try { | ||
| const existingContent = fs.readFileSync(CONFIG_FILE, 'utf8'); | ||
| const loadedConfig = yaml.load(existingContent) as CrowdinConfig; | ||
| if (loadedConfig) { | ||
| const { files, base_path, preserve_hierarchy, ...rest } = loadedConfig; | ||
| otherConfigProps = rest; | ||
| } | ||
| } catch (e) { console.error(' --- Error, crowdin.yaml not found ---', e); } | ||
| } | ||
|
|
||
| const filesJsonString = JSON.stringify(crowdinFiles, null, 4); | ||
| let finalOutput = `"base_path": "."\n\n"preserve_hierarchy": true\n\n`; |
There was a problem hiding this comment.
The script loads an existing Crowdin config but explicitly discards base_path and preserve_hierarchy (and then hard-codes them later). If someone has customized these values, running this script will overwrite them unexpectedly. Prefer preserving existing values when present, or make the overwrite explicit via constants/flags.
| if (fs.existsSync(CONFIG_FILE)) { | |
| try { | |
| const existingContent = fs.readFileSync(CONFIG_FILE, 'utf8'); | |
| const loadedConfig = yaml.load(existingContent) as CrowdinConfig; | |
| if (loadedConfig) { | |
| const { files, base_path, preserve_hierarchy, ...rest } = loadedConfig; | |
| otherConfigProps = rest; | |
| } | |
| } catch (e) { console.error(' --- Error, crowdin.yaml not found ---', e); } | |
| } | |
| const filesJsonString = JSON.stringify(crowdinFiles, null, 4); | |
| let finalOutput = `"base_path": "."\n\n"preserve_hierarchy": true\n\n`; | |
| let basePath: string | undefined; | |
| let preserveHierarchy: boolean | undefined; | |
| if (fs.existsSync(CONFIG_FILE)) { | |
| try { | |
| const existingContent = fs.readFileSync(CONFIG_FILE, 'utf8'); | |
| const loadedConfig = yaml.load(existingContent) as CrowdinConfig; | |
| if (loadedConfig) { | |
| const { files, base_path, preserve_hierarchy, ...rest } = loadedConfig; | |
| basePath = base_path ?? basePath; | |
| preserveHierarchy = preserve_hierarchy ?? preserveHierarchy; | |
| otherConfigProps = rest; | |
| } | |
| } catch (e) { console.error(' --- Error, crowdin.yaml not found ---', e); } | |
| } | |
| const filesJsonString = JSON.stringify(crowdinFiles, null, 4); | |
| const resolvedBasePath = basePath ?? "."; | |
| const resolvedPreserveHierarchy = preserveHierarchy ?? true; | |
| let finalOutput = `"base_path": ${JSON.stringify(resolvedBasePath)}\n\n"preserve_hierarchy": ${JSON.stringify(resolvedPreserveHierarchy)}\n\n`; |
| { | ||
| "name": "devportal-docs", | ||
| "version": "1.0.0", | ||
| "description": "This repository contains all some spaces synched from GitBook", |
There was a problem hiding this comment.
Package description text looks ungrammatical: "This repository contains all some spaces synched from GitBook". Consider rephrasing (e.g., "contains some spaces synced from GitBook") to avoid confusion in npm metadata.
| "description": "This repository contains all some spaces synched from GitBook", | |
| "description": "This repository contains some spaces synced from GitBook", |
| }, | ||
| "devDependencies": { | ||
| "@types/js-yaml": "^4.0.9", | ||
| "@types/node": "^25.2.3", |
There was a problem hiding this comment.
@types/node is pinned to v25.x while the repo’s runtime Node version is 22.18.0 (see .node-version). Using mismatched Node typings can introduce incorrect type definitions and TS compilation/runtime assumptions. Consider aligning @types/node major to the Node runtime major (22.x) unless you intentionally target Node 25 APIs.
| "@types/node": "^25.2.3", | |
| "@types/node": "^22.0.0", |
| inputs: | ||
| package_manager: | ||
| description: 'The package manager to use' | ||
| default: 'npm' | ||
| required: true | ||
|
|
There was a problem hiding this comment.
This composite action defines an input package_manager, but it’s not referenced in any step. Either wire it into the action (e.g., to choose npm/pnpm/yarn setup) or remove the unused input to avoid confusing callers.
| inputs: | |
| package_manager: | |
| description: 'The package manager to use' | |
| default: 'npm' | |
| required: true |
PR For the crowdin POC
This adds node to the repository
In order to work, this should be merged in main (the triggered workflow runs from that branch)
If this is an issue, we can try and make it a manual workflow
In order for the workflow to run correctly, theese variables must be added to the github repo: