-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Slight Changes to Code #16921
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
Slight Changes to Code #16921
Conversation
Hi @tiagosampaio. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
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.
Look at unit tests - they are failing. Please adjust them
@@ -43,9 +43,10 @@ public function __construct( | |||
$this->blockRepository = $blockRepository; | |||
$this->jsonFactory = $jsonFactory; | |||
} | |||
|
|||
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.
Please revert change on this line
@@ -54,9 +54,10 @@ public function __construct( | |||
$this->pageRepository = $pageRepository; | |||
$this->jsonFactory = $jsonFactory; | |||
} | |||
|
|||
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.
Please revert change on this line
@ihor-sviziev the fixes for tests were applied and the changes were made for the files you mentioned. |
Hi @ihor-sviziev, thank you for the review. |
Hi @tiagosampaio. Thank you for your contribution. Please, consider to port this solution to 2.3 release line. |
Changes like:
addError
,addSuccess
byaddErrorMessage
,addSuccessMessage
.Description
Contribution checklist