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.

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

DYN-7480 Merge master_cl-localization into master_cl #15493

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

escbuild
Copy link
Collaborator

@escbuild escbuild commented Sep 19, 2024

Hi , could you please review and merge this PR?

================== Word Count Statistics begin ==================

GMT Thu Sep 19 10:06:38 2024: Localization Engineer Lead for this PR is [email protected],[email protected].

Vendor Engineer for this PR is [email protected].

Localization branch is updated.
New/Updated strings status
CHS: all strings are translated
CHT: all strings are translated
CSY: all strings are translated
DEU: all strings are translated
ESP: all strings are translated
FRA: all strings are translated
ITA: all strings are translated
JPN: all strings are translated
KOR: all strings are translated
PLK: all strings are translated
PTB: all strings are translated
RUS: all strings are translated

Please note that If the word count above shows some words in English, it is because the localization team hasn't processed all the new/updated strings yet. These words will be translated in the next translation loop.

Please merge this PR anyway as it will align the localizable file(s) content with the English file(s) content and prevent any functionality issue caused from misaligned localized file(s) versus English file(s).

If you are about to release and you are concerned about these strings showing up in English, please contact your localization contact/project team. Thanks.

=================== Word Count Statistics end ==================

From build https://ls.jenkins.autodesk.com/job/CL/job/DynamoCore_CL/2/
Professional translation: all languages
Pseudo translation: none
@escbuild escbuild force-pushed the master_cl-localization branch from b48249f to d82651c Compare September 19, 2024 10:06
@RobertGlobant20
Copy link
Contributor

RobertGlobant20 commented Oct 4, 2024

Hi Team, About this changes I have two questions:

  1. The resources were added manually using Visual Studio (click right over the Properties folder -> Add -> New Item -> Resources File), right?
  2. I can see that the Access Modifier for the Resources is "No code generation" (see image below) while most of the resource files inside Dynamo solucion have the Access Modifier = Public, was this intentional?
    image

Thanks

@QilongTang
Copy link
Contributor

@lilian-rossi Are you able to answer the questions above?

@ZbynekHanak-RWS
Copy link
Collaborator

Hi @RobertGlobant20,

  1. No, these localized files were not added via Visual Studio. CL takes as source the Resources.en-US.resx files, process it in our translation tool Passolo (replacing entries with translations) and generates the localized Resources.*.resx.
  2. No Visual Studio related settings is done in CL, I believe the settings for the localized Resources.*.resx files should be the same to Resources.en-US.resx file.

Thank you

@RobertGlobant20
Copy link
Contributor

RobertGlobant20 commented Oct 9, 2024

Hi @RobertGlobant20,

  1. No, these localized files were not added via Visual Studio. CL takes as source the Resources.en-US.resx files, process it in our translation tool Passolo (replacing entries with translations) and generates the localized Resources.*.resx.
  2. No Visual Studio related settings is done in CL, I believe the settings for the localized Resources.*.resx files should be the same to Resources.en-US.resx file.

Thank you

@ZbynekHanak-RWS just by chance do you know how the Dynamo solution is built by the CI job using the resources that you added? or did you tested that your changes were working for other languages?

I'm asking because due that you didn't use Visual Studio for adding the .resx file (so the tag < EmbeddedResources > is not added in the .csproj file) I'm not sure how we are managing to build the Dynamo solution and use the resources added when Dynamo is in a different language.

Just for testing purposes in my local dev setup added the files Resources.es-ES.resx and Resources.fr-FR.resx in the path ....Dynamo\src\DynamoCoreWpf\Properties the first one was added using VS2022 and the other one was added using an external tool (an app that just creates the .resx file in a specific location) but when using fr-FR the value was NOT found and used the default one so that's why I guess during the build process we are doing something specific for indicating the location of resx files in different language than english but not sure.

@ZbynekHanak-RWS
Copy link
Collaborator

Hi @RobertGlobant20 , unfortunately, I have no visibility how the Dynamo solution works. Seems this is not connected CI/solution build and localized files delivery from CL. Looks to build localized resources these localized .resx need to be added to the solution by Visual Studio first. CL process then just monitors changes in sources - Resource.en-US.resx files and delivers aligned/synced localized Resource.*.resx files. Thanks

@QilongTang QilongTang changed the title Merge master_cl-localization into master_cl DYN-7480 Merge master_cl-localization into master_cl Oct 10, 2024
@QilongTang
Copy link
Contributor

@QilongTang
Copy link
Contributor

We are ready to consume this change but ran into some pipeline problem

@QilongTang
Copy link
Contributor

QilongTang commented Oct 11, 2024

Since the PR checks keeps failing and the errors are related to some out of date dependencies, merging master into this branch to give it another try. Will merge once all checks passing

I guess PR checks are not mandate for this since this PR does not target master branch, merging

@QilongTang QilongTang added this to the 3.4 milestone Oct 11, 2024
Copy link

UI Smoke Tests

Test: success. 11 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests

@QilongTang QilongTang force-pushed the master_cl-localization branch from 5526d80 to d82651c Compare October 11, 2024 14:56
@RobertGlobant20 RobertGlobant20 merged commit 9a82c17 into master_cl Oct 11, 2024
32 of 41 checks passed
@RobertGlobant20 RobertGlobant20 deleted the master_cl-localization branch October 11, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants