Remove forbidden @author tag from Magento_Captcha and Magento_Cms#36980
Remove forbidden @author tag from Magento_Captcha and Magento_Cms#36980fredden wants to merge 11 commits intomagento:2.4-developfrom
@author tag from Magento_Captcha and Magento_Cms#36980Conversation
|
Hi @fredden. Thank you for your contribution! Add the comment under your pull request to deploy test or vanilla Magento instance:
❗ Automated tests can be triggered manually with an appropriate comment:
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
|
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
|
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
|
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
| return $block; | ||
| } catch (NoSuchEntityException $e) { | ||
| } | ||
| $storeId = $this->_storeManager->getStore()->getId(); |
There was a problem hiding this comment.
@fredden I think this change could have influence on current implementation. Or you are sure that NoSuchEntityException never is thrown? If you are i'll approve it.
There was a problem hiding this comment.
I have restored the existing code here to ensure that this pull request does not inadvertently change behaviour. I initially removed it because the linter detected the empty catch block and complained. I didn't see a good reason at the time to keep the block. I agree that removing it is a behaviour change which is out of scope for this particular pull request. I'll let others fix the code when they are ready to do so. Thanks for highlighting this out-of-scope change.
|
Adding the same priority as #36976 (comment) |
|
Hello @fredden , Thanks for the contributions! Please resolve the conflict so we can proceed with this PR. Thanks. |
|
I should be able to sort this out in the next few days. |
|
@magento run all tests |
|
Hello @fredden, Thanks for resolving the conflicts. I have fixed the static failure, but there is one review comment that needs to be resolved before proceeding further. Could you please address #36980 (comment)? Thanks again! |
|
@magento run all tests |
Hello @fredden, This is a reminder: Could you please address #36980 (comment)? Thanks. |
|
@magento run all tests |
|
@engcom-Dash I have adjusted the code and requested another review from Leon. |
|
Hello @leonhelmus, It looks like @fredden has responded to your review comment. Could you please take a look and provide your feedback? Thanks! |
|
@magento run all tests |
Description
According to https://devdocs.magento.com/guides/v2.4/coding-standards/docblock-standard-general.html#documentation-space, the
@authortag is not permitted in Magento. This pull request removes this tag from theMagento_CaptchaandMagento_Cmsmodules. Given there are so many instances of this tag, I've opened a small pull request to get the process started. I expect that the linter will force me to fix several other coding standards violations on the way, so having a smaller pull request means that task is easier to manage. I plan to open more pull requests to tackle the other instances of this tag.See also magento/magento-coding-standard#382 and magento/magento-coding-standard#167
See also #36304, #36976, #36977, #36978, #36979
Manual testing scenarios
There are not code changes in this pull request. This pull request only removes forbidden comments.
Contribution checklist