-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
[PHPDOC] Fix bad phpdoc Magento\Framework\App\Config\ScopeConfigInterface #39199
base: 2.4-develop
Are you sure you want to change the base?
[PHPDOC] Fix bad phpdoc Magento\Framework\App\Config\ScopeConfigInterface #39199
Conversation
Hi @dimitriBouteille. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
@magento run all tests |
@magento run all tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @dimitriBouteille,
Thank you for your contribution!
The changes look good to us. However, please check the failed tests, particularly the static test, and address the issues accordingly.
@magento run all tests |
@magento run Static Tests |
@engcom-Hotel- I have fixed the static failures. Moving back to pending review. |
@magento run all tests |
@engcom-Dash Thanks you ❤️ |
@engcom-Bravo @engcom-Hotel Why some tests fails ? Same problem here #39153 I think the problem don't come from this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @dimitriBouteille,
Please refer to my below review comment
Thanks @engcom-Dash for highlighting the same.
Thanks
lib/internal/Magento/Framework/App/Config/ScopeConfigInterface.php
Outdated
Show resolved
Hide resolved
lib/internal/Magento/Framework/App/Config/ScopeConfigInterface.php
Outdated
Show resolved
Hide resolved
@magento run all tests |
@magento run all tests |
Manual testing is not required here, as this PR is only updating the doc block. Since the builds are failing, moving it to Extended Testing. |
@magento run Unit Tests, Functional Tests CE |
@magento create issue |
The Functional CE test failures are inconsistent in the last two builds and seem to be flaky. They neither part of PR nor failing because of the PR changes, hence moving it to Merge in Progress. Run 1: https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/39199/a125ed7085d0b92289c951805b6e9521/Functional/allure-report-ce/index.html#categories/bdbf199525818fae7a8651db9eafe741 Run 2: https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/39199/f392145369943d85ff0af0463c0324f9/Functional/allure-report-ce/index.html#categories/ee9a7904470df46f41d453cf2e3cad02/b03e5f18341f5c1f/ |
Description (*)
This PR fix bad phpdoc in
Magento\Framework\App\Config\ScopeConfigInterface
:getValue
: The$scopeCode
argument can accept StoreInterfaceisSetFlag
: The$scopeCode
argument can accept StoreInterfaceHere is an example where the type
StoreInterface
is used :magento2/lib/internal/Magento/Framework/App/Config.php
Lines 66 to 75 in 7377de5
For example, with PHPSTAN is very complicated to setup the project with level 5 or higher :(
Related Pull Requests
None
Fixed Issues (if relevant)
None
Manual testing scenarios (*)
Setup PHPSTAN with level 5 or higher and run check.
Contribution checklist (*)
Resolved issues: