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 missing translation domains #13605

Merged

Conversation

@matks
Copy link
Contributor

matks commented Apr 29, 2019

Questions Answers
Branch? 1.7.6.x
Description? Add missing translation domains
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? Please check that Import migrated page is alright as well as Database Manager migrated page

This change is Reviewable

@matks matks requested a review from PrestaShop/prestashop-core-developers as a code owner Apr 29, 2019
Copy link
Contributor

PierreRambaud left a comment

Code looks legit, waiting for wording.

Copy link
Contributor

jolelievre left a comment

I didn't comment them all but a few calls have too many parameters (domain already present)
And I'm wondering about the use of Admin.Advparameters.Feature averywhere? Is this because it's for the Import feature? Maybe some of these keys already exist in other domain nonetheless

new EntityField('name', $this->trans('Name', 'Admin.Global')),
new EntityField('address1', $this->trans('Address', 'Admin.Global'), '', true),
new EntityField('address2', $this->trans('Address (2)')),
new EntityField('id', $this->trans('ID', 'Admin.Global', 'Admin.Advparameters.Feature')),

This comment has been minimized.

Copy link
@jolelievre

jolelievre Apr 29, 2019

Contributor

This one seems weird.. I think it's the second parameters array which is missing

Suggested change
new EntityField('id', $this->trans('ID', 'Admin.Global', 'Admin.Advparameters.Feature')),
new EntityField('id', $this->trans('ID', [], 'Admin.Global')),

This comment has been minimized.

Copy link
@jolelievre

jolelievre Apr 29, 2019

Contributor

My bad, there is no array parameter in this case, however the domain parameter was already provided

This comment has been minimized.

Copy link
@PierreRambaud

PierreRambaud Apr 30, 2019

Contributor

yep, because of

private function trans($id, $domain = 'Admin.Advparameters.Feature')
(in case someone see your comments)

new EntityField('price_tex', $this->trans('Price tax excluded')),
new EntityField('price_tin', $this->trans('Price tax included')),
new EntityField('id_tax_rules_group', $this->trans('Tax rule ID')),
new EntityField('id', $this->trans('ID', 'Admin.Global', 'Admin.Advparameters.Feature')),

This comment has been minimized.

Copy link
@jolelievre

jolelievre Apr 29, 2019

Contributor
Suggested change
new EntityField('id', $this->trans('ID', 'Admin.Global', 'Admin.Advparameters.Feature')),
new EntityField('id', $this->trans('ID', 'Admin.Global')),
Copy link
Contributor

PierreRambaud left a comment

Because @jolelievre has a better eyes 😄

@jolelievre

This comment has been minimized.

Copy link
Contributor

jolelievre commented Apr 30, 2019

Because @jolelievre has a better eyes 😄

My eyes were WIDE open after the last game of throne's episode..

@matks

This comment has been minimized.

Copy link
Contributor Author

matks commented Apr 30, 2019

Because @jolelievre has a better eyes 😄

My eyes were WIDE open after the last game of throne's episode..

🐉 ⚔️

@matks

This comment has been minimized.

Copy link
Contributor Author

matks commented Apr 30, 2019

Fixed the domain duplicates, now waiting for wording validation

@eternoendless eternoendless added this to the 1.7.6.0 milestone Apr 30, 2019
Apply wording feedbacks

Co-Authored-By: matks <mathieu.ferment@prestashop.com>
@matks matks dismissed stale reviews from jolelievre and PierreRambaud May 2, 2019

Requested changes have been applied when eligible

@matks

This comment has been minimized.

Copy link
Contributor Author

matks commented May 2, 2019

@LouiseBonnard applied your changes
@jolelievre please re-review 😄

@eternoendless

This comment has been minimized.

Copy link
Member

eternoendless commented May 2, 2019

Thank you @matks

@eternoendless eternoendless merged commit 8c0973a into PrestaShop:1.7.6.x May 2, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@matks matks deleted the matks:add-missing-translation-domains branch Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.