-
Notifications
You must be signed in to change notification settings - Fork 676
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
Move package.nls.*.json to root #6118
Move package.nls.*.json to root #6118
Conversation
package.json
Outdated
@@ -70,7 +70,7 @@ | |||
"unpackage:vsix": "gulp vsix:release:unpackage", | |||
"updatePackageDependencies": "gulp updatePackageDependencies", | |||
"l10nDevGenerateXlf": "npx @vscode/l10n-dev generate-xlf ./package.nls.json ./l10n/bundle.l10n.json --outFile ./loc/vscode-csharp.xlf", | |||
"l10nDevImportXlf": "npx @vscode/l10n-dev import-xlf ./loc/vscode-csharp.*.xlf --outDir ./l10n" | |||
"l10nDevImportXlf": "npx @vscode/l10n-dev import-xlf ./loc/vscode-csharp.*.xlf --outDir ./l10n && move /l10n/package.nls.*.json ." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'move' can't be used on bash. (mv should be there)
But for package.nls.json, if a contributor makes a change in package.json and creates a reference to it. That's everything.
This task should not be run on dev's machine. It should only be run on CI (windows environment, because localization tasks can only be run there)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is currently linux based I believe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just specify '.' as the output dir so we don't have to move it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, localization CI is windows see
vscode-csharp/azure-pipelines/loc.yml
Line 20 in aab2804
demands: ImageOverride -equals windows.vs2022preview.amd64 |
OneLocBuild Task is required to run on windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just specify '.' as the output dir so we don't have to move it?
Yeah but in that case we need to move bundle.l10.*.json
to l10
folder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or it's can be something like this, we put all bundle.l10.*.json
and package.nls.*.json
to root. And change this
Line 46 in aab2804
"l10n": "./l10n", |
I just feel it's too many things under the root
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but in that case we need to move bundle.l10.*.json to l10 folder
Oooh ok yeah can leave it there and move it
OneLocBuild Task is required to run on windows
Ah this runs in the separate pipeline - my bad.
Looks like vs code can only load package.nls.*json from root dir.
Verified: