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.

Sign up for GitHub

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

Jump to bottom

Task/jenkins 35845 i18n key rename #579

Merged
merged 9 commits into from
Nov 7, 2016
Merged

Task/jenkins 35845 i18n key rename #579

merged 9 commits into from
Nov 7, 2016

Conversation

cliffmeyers
Copy link
Contributor

Description

  • See JENKINS-35845.
  • Merging this to @scherler branch so he can see more easily what I changed. Figured this was easier than just committing directly to his branch.

@cliffmeyers
Copy link
Contributor Author

@scherler we should get this merged to your branch sooner rather than later before there is drift in the code.

scherler
scherler approved these changes Nov 7, 2016
View reviewed changes
Copy link
Collaborator

@scherler scherler left a comment

Choose a reason for hiding this comment

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

Looks goods, I will merge it and implement my comments.

Sorry for not attending this earlier, but with vacation and conference I did not found the time.

blueocean-dashboard/src/main/js/components/Activity.jsx
@@ -88,23 +88,24 @@ export class Activity extends Component {
}

const latestRun = runs[0];
const head = 'pipelinedetail.activity.header';
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not move this to top level and use

t(`${head}.xxx`)

all the time (especially the one above)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll admit I am not sure which approach is the best. Part of me felt that using the full keys in all places would make it easier to search across the code base and see where certain keys were used without having to search for substrings of the keys. On the other hand, it is a little verbose and repetitive in the code. Curious what the the rest of the team feels about this, cc @imeredith @kzantow @tfennelly @sophistifunk @michaelneale

blueocean-dashboard/src/main/resources/jenkins/plugins/blueocean/dashboard/Messages.properties
@@ -1,23 +1,81 @@
Activity=Activity
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO it makes sense to have a strict alphabetic order for the keys of our properties. Meaning we should do something like

sort -t\  -n Messages.properties > Messages.properties_sort
#review
mv Messages.properties_sort Messages.properties

on them before commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, my unix-fu isn't up to snuff. Can you describe the desired end state?

Copy link
Collaborator

Choose a reason for hiding this comment

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

just sorting the file using the key column

Copy link
Collaborator

Choose a reason for hiding this comment

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

@cliffmeyers do not worry will do it in JENKINS-35845

Merge branch 'JENKINS-35845' into task/JENKINS-35845-i18n-key-rename
b8dfec6 
# Conflicts:
#	blueocean-dashboard/src/main/js/components/Pipelines.jsx
@scherler scherler merged commit cd726f8 into JENKINS-35845 Nov 7, 2016
@scherler scherler deleted the task/JENKINS-35845-i18n-key-rename branch November 7, 2016 15:21
scherler added a commit that referenced this pull request Nov 17, 2016
@scherler
[FIX JENKINS-39225] Internationalisation for Blue Ocean and JDL (#556)
ed76c2c 
* [JENKINS-35845] WIP first steps with i18n

* eslint - formating changes and fix offences

* [JENKINS-35845] first basic working version with 2 languages

* [JENKINS-35845] WIP adding patched backend to investigate

* [JENKINS-35845] Assumes jenkinsci/jenkins#2586 to be applied. We know get the translations from the standard jenkins way, but needs changes in the core for now

* [JENKINS-35845] remove testing class

* [JENKINS-35845] WIP fixing integration of jenkins i18n keys with dot in them

* [JENKINS-35845] move i18n to core-js and using it from within web

* [JENKINS-35845] follow jenkins convention/pattern for storing locale

* [JENKINS-35845] WIP implement translation in core-js runbutton and toastUtil. Prefix translations with bo.web. remove sample code

* [JENKINS-35845] WIP starting to translate dashboard

* [JENKINS-35845] WIP added german translation and finished dashboard. Currently working on making moment respect the locale

* [JENKINS-35845] Use latest jdl

* [JENKINS-35845] fix locale retrieval

* [JENKINS-35845] update stories to use the new i18n functions. Create story to test readableDate and timeDuration. In node this works fine now traking down wht not in dashboard

* [JENKINS-35845] Add spanish translations

* [JENKINS-35845] better translation for spanish (thank you @Dario) as well updated some german translations

* [JENKINS-35845] Fix german translations with feedback from @daniel

* [JENKINS-35845] Fix test view and finish translation

* [JENKINS-35845] Fix german translation. fix result views. fix some lint issues.

* [JENKINS-35845] create a compose function to wire different functions together

* [JENKINS-35845] better documentation and remove debug string

* [JENKINS-35845] add documentation about i18n and linking contributing and i18n docu in principal readme.

* [JENKINS-35845] updte docu

* [JENKINS-35845] Pass locale and translation function down the component tree. Fix links to not drop query parameters.

* [JENKINS-35845] Fix links to not drop query

* [JENKINS-35845] use the url config util to get correct path to jenkins

* [JENKINS-35845] WIP security commit - refactor i18n class to support listener and subscribe to i18nChanges. Will allow to drop react specific integration via react-i18next.

* [JENKINS-35845] WIP security commit

* [JENKINS-35845] remove debug statement

* [JENKINS-35845] fix import

* Task/jenkins 35845 i18n key rename (#579)

* [JENKINS-35845] rework i18n keys for home page

* [JENKINS-35845] l10n for activity tab

* [JENKINS-35845] l10n for branches tab

* [JENKINS-35845] l10n for pull requests tab

* [JENKINS-35845] l10n for run details -> pipeline

* [JENKINS-35845] l10n for run details -> changes, tests, artifacts

* [JENKINS-35845] l10n for run details header changes

* [JENKINS-35845] pagination; fix typo causing error

* [JENKINS-35845] order keys

* [JENKINS-35845] WIP security commit to be able to merge master

* eslint - formating changes and fix offences

* [JENKINS-35845] Remove dep to snapshot-jenkins and implement fallback to default values. Fix tests of all related projects.

* [JENKINS-35845] remove duplicate variable

* [JENKINS-35845] fix test by adding polyfy again

* eslint - formating changes and fix offences

* [JENKINS-35845] Fix last test

* [JENKINS-35845] fix tests for dashboard

* [JENKINS-35845] sync version numbers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scherler scherler scherler approved these changes

Assignees
No one assigned
Labels
DO NOT MERGE work-in-progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cliffmeyers @scherler @michaelneale