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

Move more i18n data translations to Intl plugin #8101

Merged
merged 12 commits into from
Jun 22, 2015
Merged

Move more i18n data translations to Intl plugin #8101

merged 12 commits into from
Jun 22, 2015

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jun 12, 2015

matomo-org/plugin-CustomAlerts#16 needs to be merged afterwards

@sgiehl sgiehl mentioned this pull request Jun 12, 2015
Closed
@sgiehl sgiehl added Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Jun 12, 2015
@tsteur
Copy link
Member

tsteur commented Jun 14, 2015

I can imagine those strings might be used by a couple of other plugins? Especially Piwik PRO plugins but also other plugins maybe?

Would it make maybe sense to deprecate those keys first before completely removing them? Might be overdoing it, just a question. Would be otherwise at least worth mentioning in the developer changelog.

Only did a check for three strings. General_Seconds is eg also used in Piwik-API repository. CoreHome_PeriodWeek is eg used in Piwik Mobile, Piwik-API, UserCountryMap_Minutes is used in the Piwik OpenStreetMap plugin, ... so at least a mention in the changelog would be appropriate maybe? Not sure if we would have to list all changes or just link to this PR or so?

@mnapoli
Copy link
Contributor

mnapoli commented Jun 15, 2015

The move looks good.

For the question of backward compatibility it seems there's a piece of code to handle that in Translator (see the top of the diff), so that looks good to me. I'm +1 with that but we need to remove that code in 3.0, it would be good to create an issue about it maybe? (and mention v3.0 in the @todo maybe that way if we stumble on it later we know we can remove it)

@sgiehl
Copy link
Member Author

sgiehl commented Jun 15, 2015

I've updated the comment in Translator class and added a note to the changelog

@sgiehl
Copy link
Member Author

sgiehl commented Jun 15, 2015

@tsteur regarding the translations used in Piwik mobile. I've got an idea how to handle that a better way, but I'll create a ticket therefor. See matomo-org/matomo-mobile-2#5343

@tsteur
Copy link
Member

tsteur commented Jun 15, 2015

Just wondering: Is it easily possible to automatically generate a list of added and removed translation keys for a certain release? Could be nice for the future

@sgiehl
Copy link
Member Author

sgiehl commented Jun 15, 2015 via email

@tsteur
Copy link
Member

tsteur commented Jun 15, 2015

I'm just wondering where to list them then. Wouldn't want to bloat the changelog with them :)

@mattab
Copy link
Member

mattab commented Jun 17, 2015

@sgiehl FYI in an attempt to clarify, created: #8125 (comment)

I suggested Translation keys, especially generic ones such as General_* and CoreHome_* keys, are part of the API and should not change. as suggestion. What do you think?

@mattab mattab added this to the 2.14.0 milestone Jun 18, 2015
@mattab
Copy link
Member

mattab commented Jun 18, 2015

added to 2.14.0 milestone

@sgiehl
Copy link
Member Author

sgiehl commented Jun 18, 2015

Making some of the translations a "part of the API" sound good. But we should consider which keys to make "permanent", so we do not have to change them frequently.

@tsteur
Copy link
Member

tsteur commented Jun 18, 2015

I reckon it's good to move more and more translation keys to an Intl repository so as long as this one follows Semver we won't break anything anyway (most likely g). So might be good to move even more into this repo before 3.0 and then we can just say all those starting with Intl_ are considered API.

For the Piwik specific keys like "Websites", "Super User", ... we could maybe use a different category like Piwik_.... Piwik_ and API_ would be not really good naming I reckon since we also have classes for that and it might be confusing to see it in code. Maybe there would be a better name? Or otherwise we use General_ and move the ones from General_ that are not considered API somewhere else.

This way it would be very clear for core developers and plugin developers when a translation key becomes API and which keys won't change etc. Eg we could just write "All translation keys that start with Intl_ or General_ are considered API, all other ones can change over time even though it is rather unlikely". Or something similar...

@sgiehl
Copy link
Member Author

sgiehl commented Jun 20, 2015

I agree with @tsteur
Guess we should create a new ticket for that. Also we maybe should afterwards adjust the translation search in piwik to show which keys are "static" and which might change...

@mattab
Copy link
Member

mattab commented Jun 22, 2015

Hi @sgiehl sounds good. Do you mind create follow up issue, and merge the PR?

@sgiehl sgiehl mentioned this pull request Jun 22, 2015
Closed
sgiehl pushed a commit that referenced this pull request Jun 22, 2015
Merge pull request #8101 from piwik/i18n-data2
0dad68b 
Move more i18n data translations to Intl plugin
@sgiehl sgiehl merged commit 0dad68b into master Jun 22, 2015
@sgiehl sgiehl deleted the i18n-data2 branch June 22, 2015 11:13
mamash pushed a commit to TritonDataCenter/pkgsrc-wip that referenced this pull request Jul 12, 2015
Update 2.13.0 to 2.14.0
c2da7eb 
-----------------------
## Piwik 2.14.0

### Breaking Changes
* The `UserSettings` API has been removed. The API was deprecated in
  earlier versions. Use `DevicesDetection`, `Resolution` and
  `DevicePlugins` API instead.

* Many translations have been moved to the new Intl plugin. Most of them
  will still work, but please update their usage. See
  matomo-org/matomo#8101 for a full list

### New features
* The JavaScript Tracker does now track outlinks and downloads if a user
  opens the context menu if the `enabled` parameter of the
  `enableLinkTracking()` method is set to `true`. To use this new feature
  use `tracker.enableLinkTracking(true)` or
  `_paq.push(['enableLinkTracking', true]);`. This is not industry
  standard and is vulnerable to false positives since not every user will
  select "Open in a new tab" when the context menu is shown. Most users
  will do though and it will lead to more accurate results in most cases.

* The JavaScript Tracker now contains the 'heart beat' feature which can
  be used to obtain more accurate visit lengths by periodically sending
  'ping' requests to Piwik. To use this feature use
  `tracker.enableHeartBeatTimer();` or
  `_paq.push(['enableHeartBeatTimer']);`. By default, a ping request will
  be sent every 15 seconds. You can specify a custom ping delay (in
  seconds) by passing an argument, eg,
  `tracker.enableHeartBeatTimer(10);` or
  `_paq.push(['enableHeartBeatTimer', 10]);`.

* New custom segment `languageCode` that lets you segment visitors that
  are using a particular language. Example values: `de`, `fr`, `en-gb`,
  `zh-cn`, etc.

* Segment `userId` now supports any segment operator (previously only
  operator Contains `=@` was supported for this segment).

### Commands updates
* The command `core:archive` now has two new parameter:
  `--force-idsegments` and `--skip-idsegments` that let you force (or
  skip) processing archives for one or several custom segments.

* The command `scheduled-tasks:run` now has an argument `task` that lets
  you force run a particular scheduled task.

### Library updates

* Updated pChart library from 2.1.3 to 2.1.4. The files were moved from
  the directory `libs/pChart2.1.3` to `libs/pChart`

### Internal change

* To execute UI tests "ImageMagick" is now required.
* The Q JavaScript promise library is now distributed with tests and can
  be used in the piwik.js tests.
@mattab mattab added the c: i18n For issues around internationalisation and localisation. label Oct 13, 2015
@mattab mattab added the answered For when a question was asked and we referred to forum or answered it. label Oct 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
answered For when a question was asked and we referred to forum or answered it. c: i18n For issues around internationalisation and localisation. Needs Review PRs that need a code review
Projects
None yet
Milestone
2.14.0
Development

Successfully merging this pull request may close these issues.
4 participants
@sgiehl @tsteur @mnapoli @mattab