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 Chinese translation for global configuration #3355

Merged
merged 4 commits into from Apr 21, 2018

Conversation

@LinuxSuRen
Copy link
Member

LinuxSuRen commented Mar 17, 2018

@@ -0,0 +1,26 @@
# The MIT License
#
# Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck Mar 17, 2018

Member

Wrong copyright.

Copy link
Member

daniel-beck left a comment

Should not be merged before existing translations have been migrated to the f:apply control as discussed in #3287.

@@ -62,7 +62,7 @@ THE SOFTWARE.

<f:bottomButtonBar>
<f:submit value="${%Save}" />
<f:apply value="${%Apply}"/>
<f:apply />

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck Mar 17, 2018

Member

While the followup change to #3287 is not filed, this will remove existing translations without replacement.

This comment has been minimized.

Copy link
@LinuxSuRen

LinuxSuRen Mar 17, 2018

Author Member

#3287 already been merged.

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck Mar 17, 2018

Member

Yes, without moving the existing localizations to the f:apply control. It's similar to the discussion in #3319.

If we merge this change, system configuration will lose the apply button localization for Bulgarian, German, French, Italian, etc., as there are no corresponding translations to https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/lib/form/apply.jelly -- only English and Chinese.

This comment has been minimized.

Copy link
@LinuxSuRen

LinuxSuRen Mar 17, 2018

Author Member

It's my mistake. So your option is put them back, or remove control to f:apply.

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck Mar 17, 2018

Member

In my opinion, the preferable option --as in #3319 -- would be to just move the translations to apply_*.properties files.

If you don't wish to work on translations of languages you don't know, keeping the translations around and reverting part of this PR would also work.

This comment has been minimized.

Copy link
@LinuxSuRen

LinuxSuRen Mar 17, 2018

Author Member

Ok than, I move those files.

LinuxSuRen added 2 commits Mar 17, 2018
@LinuxSuRen
Copy link
Member Author

LinuxSuRen commented Apr 7, 2018

@daniel-beck How do I should to deal with this pr?

@daniel-beck daniel-beck removed the on-hold label Apr 7, 2018
@oleg-nenashev
Copy link
Member

oleg-nenashev commented Apr 7, 2018

We will re-review it today

@daniel-beck
Copy link
Member

daniel-beck commented Apr 7, 2018

Will need to re-review the localization situation after the linked PR was merged.

@LinuxSuRen
Copy link
Member Author

LinuxSuRen commented Apr 21, 2018

Does something block this PR merge into master?
@daniel-beck

Copy link
Member

daniel-beck left a comment

Applys seem to have been restored.

@daniel-beck daniel-beck removed their assignment Apr 21, 2018
@daniel-beck daniel-beck merged commit 398e032 into jenkinsci:master Apr 21, 2018
2 checks passed
2 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.