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

Update DataProvider.php #8217

Merged
merged 7 commits into from
Feb 11, 2017
Merged

Update DataProvider.php #8217

merged 7 commits into from
Feb 11, 2017

Conversation

redelschaap
Copy link
Contributor

@redelschaap redelschaap commented Jan 20, 2017

Some template files from extensions cause the js-translation.json file to contain an empty array. The try-catch block prevents this and will just exclude that specific file from the loaded language strings, since it's almost impossible to debug which file is causing this.

Some template files from extensions cause the js-translation.json file to contain an empty array. The try-catch block prevents this and will just exclude that specific file from the loaded language strings, since it's almost impossible to debug which file is causing this.
@vrann vrann self-assigned this Feb 3, 2017
@vrann vrann self-requested a review February 3, 2017 16:57
Copy link
Contributor

@vrann vrann left a comment

Choose a reason for hiding this comment

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

Can you please log the Exception, so that the issue with the improper template will be flagged to the developer

@vrann
Copy link
Contributor

vrann commented Feb 3, 2017

@redelschaap Thanks for contribution, please see comment above

@redelschaap
Copy link
Contributor Author

redelschaap commented Feb 6, 2017

@vrann I've made some changes.

@@ -109,6 +109,8 @@ public function getData($themePath)
$dictionary[$phrase] = $translatedPhrase;
}
} catch (\Exception $e) {
throw new \Exception(sprintf('Error while translating phrase "%s" in file %s.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use \Magento\Framework\LocalizedException (or create subclass of it)

@vrann
Copy link
Contributor

vrann commented Feb 6, 2017

@redelschaap Thanks! Added one more comment. And please fix static test failed in Travis.

@redelschaap
Copy link
Contributor Author

redelschaap commented Feb 7, 2017

@vrann I updated the code. I cannot fix the issue in Travis since those issues are out of the scope of my adjustments.

@vrann
Copy link
Contributor

vrann commented Feb 7, 2017

@redelschaap changes look good.
This static test failure indicates wrong formatting in the catch clause:

FILE: ...magento/magento2/app/code/Magento/Translation/Model/Js/DataProvider.php
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 114 | ERROR | Opening parenthesis of a multi-line function call must be the
     |       | last content on the line
 115 | ERROR | Only one argument is allowed per line in a multi-line function
     |       | call
 115 | ERROR | Closing parenthesis of a multi-line function call must be on a
     |       | line by itself
--------------------------------------------------------------------------------

Can you please fix those?

@redelschaap
Copy link
Contributor Author

Made the adjustments, Travis still failed but that is out of my scope:

1) Magento\Test\Php\LiveCodeTest::testCodeMess
PHP Code Mess has found error(s):
/home/travis/build/magento/magento2/app/code/Magento/Translation/Model/Js/DataProvider.php:14	The class DataProvider has a coupling between objects value of 13. Consider to reduce the number of dependencies under 13.

@vrann
Copy link
Contributor

vrann commented Feb 8, 2017

@redelschaap thanks, that was everything needed

@okorshenko okorshenko added this to the February 2017 milestone Feb 11, 2017
@mmansoor-magento mmansoor-magento merged commit cd11b63 into magento:develop Feb 11, 2017
@okorshenko
Copy link
Contributor

@redelschaap thank you for your contribution to Magento 2 project. Your PR is successfully merged to develop branch

magento-devops-reposync-svc pushed a commit that referenced this pull request Jun 13, 2023
…2-cia-2.4.7-beta1-bugfixes-02062023

Revert "Revert "Cia 2.4.7 beta1 bugfixes 02062023""
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