Skip to content

Remove translations from management section#13049

Merged
epixa merged 1 commit intoelastic:masterfrom
epixa:removetranslations
Jul 24, 2017
Merged

Remove translations from management section#13049
epixa merged 1 commit intoelastic:masterfrom
epixa:removetranslations

Conversation

@epixa
Copy link
Copy Markdown
Contributor

@epixa epixa commented Jul 22, 2017 •
edited
Loading

We need to revisit how we embed translations into templates, so until we
do we might as well go back to embedding text directly in the page. The
translation implementation now makes it difficult to navigate throughout
complicated HTML.

It also suffers from an inability to flag on unused translation keys,
which creates a good deal of technical debt. Close to half of all
translation keys in the translation file in this initial pilot were
already unused since it was introduced a couple of months ago.

@epixa epixa requested review from Bargs and spalger July 22, 2017 01:52
@stacey-gammon
Copy link
Copy Markdown

stacey-gammon commented Jul 24, 2017 •
edited
Loading

For some reason the build has an error but it's not being reported. If you check the ci build you'll see:

  single test that uses esArchiver
     │ debg  Loading config file from "/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-intake/src/functional_test_runner/__tests__/fixtures/with_es_archiver/config.js"
     │ info  starting elasticsearch
       │ debg  WARN: bootstrap_checks - max virtual memory areas vm.max_map_count [65530] is too low, increase to at least [262144]
       │ debg  WARN: unicast_zen_ping - [1] failed send ping to {#zen_unicast_127.0.0.1_0#}{28IVdc5GTrqrXCIHns8Rnw}{127.0.0.1}{127.0.0.1:9300}
       │ debg  WARN: unicast_zen_ping - [1] failed send ping to {#zen_unicast_127.0.0.1_0#}{28IVdc5GTrqrXCIHns8Rnw}{127.0.0.1}{127.0.0.1:9300}
       │ debg  WARN: unicast_zen_ping - [1] failed send ping to {#zen_unicast_127.0.0.1_0#}{28IVdc5GTrqrXCIHns8Rnw}{127.0.0.1}{127.0.0.1:9300}
     │ info  starting kibana
{"type":"log","@timestamp":"2017-07-22T01:57:35Z","tags":["listening","info"],"pid":4252,"message":"Server running at http://localhost:5701"}
     │ debg   info  Config loaded
     │        info  Starting tests
     │       
     │        └-: tests
     │          └-> "before all" hook
     │            │ info  [test_archive] Loading "mappings.json"
     │          └- ✖ fail: "tests "before all" hook"
     │          │        [illegal_argument_exception] Rejecting mapping update to [.kibana] as the final mapping would have more than 1 type: [server, index-pattern, visualization, search, timelion-sheet, config, url, dashboard]
     │          │         :: {"path":"/.kibana","query":{},"body":"{\"settings\":{\"index\":{\"number_of_shards\":\"1\",\"mapper\":{\"dynamic\":\"false\"},\"number_of_replicas\":\"1\"}},\"mappings\":{\"url\":{\"dynamic\":\"strict\",\"properties\":{\"accessCount\":{\"type\":\"long\"},\"accessDate\":{\"type\":\"date\"},\"createDate\":{\"type\":\"date\"},\"url\":{\"type\":\"text\",\"fields\":{\"keyword\":{\"type\":\"keyword\",\"ignore_above\":2048}}}}},\"visualization\":{\"dynamic\":\"strict\",\"properties\":{\"description\":{\"type\":\"text\"},\"kibanaSavedObjectMeta\":{\"properties\":{\"searchSourceJSON\":{\"type\":\"text\"}}},\"savedSearchId\":

@epixa
Remove translations from management section
e090f78 
We need to revisit how we embed translations into templates, so until we
do we might as well go back to embedding text directly in the page. The
translation implementation now makes it difficult to navigate throughout
complicated HTML.

It also suffers from an inability to flag on unused translation keys,
which creates a good deal of technical debt. Close to half of all
translation keys in the translation file in this initial pilot were
already unused since it was introduced a couple of months ago.
@epixa epixa force-pushed the removetranslations branch from 5ffb89c to e090f78 Compare July 24, 2017 14:48
Bargs
Bargs approved these changes Jul 24, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@Bargs Bargs left a comment

Choose a reason for hiding this comment

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

I noticed a few unused variables.

Otherwise, LGTM

@epixa
Copy link
Copy Markdown
Contributor Author

epixa commented Jul 24, 2017

@stacey-gammon That failure is on master itself. Since the rest of the tests ran successfully, I'm going to move forward with this and work on the issue on master separately.

@Bargs I'm going to do a followup that removes angular-translate, so I'll pull all the references to it in that.

@epixa epixa merged commit 90c713f into elastic:master Jul 24, 2017
epixa added a commit that referenced this pull request Jul 24, 2017
@epixa
Remove translations from management section (#13049)
4acaf61 
We need to revisit how we embed translations into templates, so until we
do we might as well go back to embedding text directly in the page. The
translation implementation now makes it difficult to navigate throughout
complicated HTML.

It also suffers from an inability to flag on unused translation keys,
which creates a good deal of technical debt. Close to half of all
translation keys in the translation file in this initial pilot were
already unused since it was introduced a couple of months ago.
@epixa
Copy link
Copy Markdown
Contributor Author

epixa commented Jul 24, 2017

5.x 4acaf61

@epixa epixa deleted the removetranslations branch July 24, 2017 17:15
@epixa epixa mentioned this pull request Jul 24, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spalger spalger Awaiting requested review from spalger

1 more reviewer

@Bargs Bargs Bargs approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release_note:fix review v5.6.0 v6.0.0-beta1 v6.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@epixa @stacey-gammon @Bargs @jimgoodwin