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

add new language po file for zh_TW kk_KZ lb_LU #2301

Closed
wants to merge 8 commits into from

Conversation

coby2023t
Copy link

there are language request for 3 new languages in transifex but look like it didn't work. the zh_TW has full translated msgstr. kk_KZ and lb_LU are blank for translators

@CLAassistant
Copy link

CLAassistant commented Sep 8, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@cmaglie
Copy link
Member

cmaglie commented Sep 11, 2023

@g-castronovo can you confirm that these languages are already handled in Transifex?

@g-castronovo
Copy link
Contributor

@cmaglie I have authorized now ZH_tw languages now. No, content is zero percent for CLI.
IDE 2.0 zh_TW is ready

@coby2023t
Copy link
Author

took 2 days cut and paste the zh_TW msgstr to Transifex, now it is ready for pull from it.

@g-castronovo
Copy link
Contributor

@coby2023t Thank you so much for your patience and collaboration. I really appreciate it.

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Patch coverage: 55.55% and no project coverage change.

Comparison is base (3f5c0eb) 63.87% compared to head (50e25a7) 63.87%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2301   +/-   ##
=======================================
  Coverage   63.87%   63.87%           
=======================================
  Files         212      212           
  Lines       19873    19873           
=======================================
  Hits        12693    12693           
  Misses       6089     6089           
  Partials     1091     1091           
Flag Coverage Δ
unit 63.87% <55.55%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
arduino/monitor/monitor.go 41.05% <0.00%> (ø)
arduino/discovery/discovery.go 42.57% <50.00%> (ø)
arduino/libraries/librariesresolver/cpp.go 92.76% <85.71%> (ø)
arduino/builder/detector/detector.go 74.34% <100.00%> (ø)
internal/cli/cli.go 87.26% <100.00%> (ø)
legacy/builder/recipe_runner.go 76.31% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -223,7 +226,7 @@ func getDownloadURL(languageCode, downloadID string) string {
case "pending":
fallthrough
case "processing":
fmt.Printf("Current status for language %s: %s\n", languageCode, status)
fmt.Printf(tr("Current status for language %s: %s\n"), languageCode, status)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it makes sense to internationalize this internal utility package. Doing so would increase the workload for the community translators without any significant benefit for the project users in return.

Credentials in the Transifex project are required to run this, which only a few select administrators would have. This is intended to be ran automatically by the continuous distribution system, so the logs are only ever viewed in the case of the need to troubleshoot a failure. In that rare eventuality, if the person doing the troubleshooting didn't have English language fluency, they could use an automatic translation service (e.g., Google Translate) on the messages.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, these messages will never reach any arduino-cli user.

@@ -189,7 +189,7 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis
disc.incomingMessagesError = err
disc.statusMutex.Unlock()
close(outChan)
logrus.Errorf("stopped discovery %s decode loop: %v", disc.id, err)
logrus.Errorf(tr("stopped discovery %s decode loop: %v"), disc.id, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

During the initial internationalization effort, we decided that the log messages would be excluded from the internationalization scope for now. The reason is that the addition of these strings would significantly increase the workload on the community translators, while generally being of less visibility and/or importance to the users.

Even though on an individual basis there are surely specific log messages of higher importance than specific non-log strings that were internationalized, the log/non-log dichotomy provided an objective criterion for determining which strings were in scope for the initial effort.

We considered eventually expanding the i18n coverage at some time in the future after there has been good progress on the initial translation effort. However, from looking at the translation coverage on the Transifex project home page:

https://explore.transifex.com/arduino-1/arduino-cli/

it seems to me that the translation coverage is not very good overall so I don't think it is a good idea to increase the translator workload by internationalization of log messages unless there are specific cases where a message is highly visible to a significant number of users for some reason.

@@ -206,7 +206,7 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis
closeAndReportError(err)
return
}
logrus.Infof("from discovery %s received message %s", disc.id, msg)
logrus.Infof(tr("from discovery %s received message %s"), disc.id, msg)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logrus.Infof(tr("from discovery %s received message %s"), disc.id, msg)
logrus.Infof(tr("from discovery %[1]s received message %[2]s"), disc.id, msg)

As mentioned above, I am opposed to the arbitrary internationalization of log messages. However, since you are interested in assisting in the internationalization effort I will add a note about how we handle this type of string:

Indices should be added to printf verbs when multiple are present in a string.

This allows the translator to easily insert them at the appropriate location in the sentence structure without being required to understand the Go printf syntax and without being restricted to their relative order in the English language source string.

@@ -253,7 +253,7 @@ func (mon *PluggableMonitor) Describe() (*PortDescriptor, error) {

// Configure sets a port configuration parameter.
func (mon *PluggableMonitor) Configure(param, value string) error {
if err := mon.sendCommand(fmt.Sprintf("CONFIGURE %s %s\n", param, value)); err != nil {
if err := mon.sendCommand(fmt.Sprintf(tr("CONFIGURE %s %s\n"), param, value)); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if err := mon.sendCommand(fmt.Sprintf(tr("CONFIGURE %s %s\n"), param, value)); err != nil {
if err := mon.sendCommand(fmt.Sprintf("CONFIGURE %s %s\n", param, value)); err != nil {

This is code, not prose, so it must not be localized:

https://arduino.github.io/arduino-cli/dev/pluggable-monitor-specification/#configure-command

The CONFIGURE command is still CONFIGURE in any locale.

@@ -273,7 +273,7 @@ func (mon *PluggableMonitor) Open(portAddress, portProtocol string) (io.ReadWrit
defer tcpListener.Close()
tcpListenerPort := tcpListener.Addr().(*net.TCPAddr).Port

if err := mon.sendCommand(fmt.Sprintf("OPEN 127.0.0.1:%d %s\n", tcpListenerPort, portAddress)); err != nil {
if err := mon.sendCommand(fmt.Sprintf(tr("OPEN 127.0.0.1:%d %s\n"), tcpListenerPort, portAddress)); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if err := mon.sendCommand(fmt.Sprintf(tr("OPEN 127.0.0.1:%d %s\n"), tcpListenerPort, portAddress)); err != nil {
if err := mon.sendCommand(fmt.Sprintf("OPEN 127.0.0.1:%d %s\n", tcpListenerPort, portAddress)); err != nil {

cmaglie pushed a commit that referenced this pull request Sep 15, 2023
Related to #2301 

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@cmaglie
Copy link
Member

cmaglie commented Sep 15, 2023

@coby2023t thanks for the PR, the missing languages have been merged through the automatic import from Transifex.
#2281

The resource files *.po are all automatically managed by tools, there is no need to manually edit or create PR for them.

About the missing translation strings, as already pointed out by @per1234:

  • We are not translating the strings from the i18n tools, because it is an internal tool that we use for development and it is not visible to the arduino-cli users.
  • The logrus messages are not translated since we are using them for debugging purposes and they are not meant to be translated (in fact they are quite challenging to translate actually if taken out-of-context).
  • Some commands must not be translated because they are instructions interpreted by other tools.

So I'm closing this PR since almost all the changes belongs to the above categories, if I'm missing something, please open another PR starting from the current master so we can review it again.

@cmaglie cmaglie closed this Sep 15, 2023
@per1234 per1234 added conclusion: duplicate Has already been submitted type: enhancement Proposed improvement conclusion: invalid Issue/PR not valid conclusion: declined Will not be worked on topic: infrastructure Related to project infrastructure labels Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: declined Will not be worked on conclusion: duplicate Has already been submitted conclusion: invalid Issue/PR not valid topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants