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

Remove translations from management section #13049

Merged
merged 1 commit into from
Jul 24, 2017

Conversation

epixa
Copy link
Contributor

@epixa epixa commented Jul 22, 2017

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.

@stacey-gammon
Copy link
Contributor

stacey-gammon commented Jul 24, 2017

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\":

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
Copy link
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
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
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
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
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