Partial docs fixes in Newsletter module #20848

ghost · 2019-01-31T10:26:53Z

Description

Small PHPDocs fixes in Newsletter
strict_type fixes in Newsletter module

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

- PHPDocs - strict_type

magento-engcom-team · 2019-01-31T10:27:02Z

Hi @SikailoISM. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento-engcom-team give me test instance - deploy test instance based on PR changes
@magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

orlangur · 2019-01-31T12:37:36Z

app/code/Magento/Newsletter/Controller/Manage/Save.php

@@ -110,7 +111,7 @@ public function execute()
                $this->messageManager->addError(__('Something went wrong while saving your subscription.'));
            }
        }
-        $this->_redirect('customer/account/');
+        return $this->_redirect('customer/account/');


Why this is needed?

I think this is logical due to lib/internal/Magento/Framework/App/ActionInterface.php:30

app/code/Magento/Newsletter/Controller/Manage/Save.php

app/code/Magento/Newsletter/Controller/Subscriber/Confirm.php

orlangur · 2019-04-24T20:36:21Z

@sidolov which sentence are you referring to? Didn't find appropriate searching by "type".

Also, these are controller actions and unlikely to be extended.

sidolov · 2019-04-29T14:59:32Z

Hi @orlangur , defining the strict return type is method signature changes actually, so, we can't make such changes for public methods according to our guides.

orlangur · 2019-04-29T19:33:29Z

@sidolov I don't see which part of the doc you're referring to. Closest one:

Some changes are detected and documented by an automated tool. These backward incompatible changes do not need manual documentation:
Modifying a method signature

For execute method I don't see why we need it as it is unlikely to be extended.

p-bystritsky · 2019-06-05T10:18:28Z

@sidolov please review.

orlangur

@SikailoISM please squash changes into a single commit so that we have perfectly clean history 😉

magento-engcom-team · 2019-06-05T15:02:50Z

Hi @sidolov, thank you for the review.
ENGCOM-4676 has been created to process this Pull Request

orlangur

Please squash changes into a single commit so that we have perfectly clean history 😉

…hpdocs-stricttypes-fixes # Conflicts: # app/code/Magento/Newsletter/Controller/Subscriber/Confirm.php

p-bystritsky · 2019-06-06T09:39:47Z

@orlangur updated.

magento-engcom-team · 2019-06-06T09:51:43Z

Hi @orlangur, thank you for the review.
ENGCOM-4676 has been created to process this Pull Request

soleksii · 2019-06-07T10:55:55Z

✔️ QA Passed

m2-assistant · 2019-06-08T22:21:48Z

Hi @SikailoISM, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

partial fixes in Newsletter module

6409dbe

- PHPDocs - strict_type

magento-engcom-team added Component: Newsletter Release Line: 2.3 Partner: ISM eCompany Pull Request is created by partner ISM eCompany partners-contribution Pull Request is created by Magento Partner labels Jan 31, 2019

orlangur self-assigned this Jan 31, 2019

orlangur requested changes Jan 31, 2019

View reviewed changes

orlangur approved these changes Apr 4, 2019

View reviewed changes

ghost added Progress: ready for testing Progress: extended testing and removed Progress: ready for testing labels Apr 4, 2019

p-bystritsky self-assigned this Apr 19, 2019

ghost added Progress: accept and removed Progress: extended testing labels Apr 23, 2019

magento-engcom-team added the Cleanup label Apr 23, 2019

sidolov requested changes Apr 24, 2019

View reviewed changes

app/code/Magento/Newsletter/Controller/Manage/Save.php Outdated Show resolved Hide resolved

app/code/Magento/Newsletter/Controller/Subscriber/Confirm.php Outdated Show resolved Hide resolved

ghost assigned sidolov Apr 24, 2019

ghost added Progress: needs update and removed Progress: accept labels Apr 24, 2019

orlangur requested changes Jun 5, 2019

View reviewed changes

sidolov approved these changes Jun 5, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Jun 5, 2019

orlangur requested changes Jun 5, 2019

View reviewed changes

ghost added Progress: needs update and removed Progress: ready for testing labels Jun 5, 2019

p-bystritsky added 2 commits June 6, 2019 12:06

Merge remote-tracking branch 'mainline/2.3-develop' into newsletter-p…

347d0d8

…hpdocs-stricttypes-fixes # Conflicts: # app/code/Magento/Newsletter/Controller/Subscriber/Confirm.php

#20848: Static test fix.

e2f577d

p-bystritsky removed the Progress: needs update label Jun 6, 2019

orlangur approved these changes Jun 6, 2019

View reviewed changes

ghost added the Progress: ready for testing label Jun 6, 2019

soleksii self-assigned this Jun 6, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Jun 6, 2019

ghost added Progress: extended testing Progress: accept and removed Progress: testing in progress Progress: extended testing labels Jun 7, 2019

magento-engcom-team merged commit e2f577d into magento:2.3-develop Jun 8, 2019

magento-engcom-team pushed a commit to okorshenko/magento2 that referenced this pull request Jun 8, 2019

ENGCOM-4676: Partial docs fixes in Newsletter module magento#20848

eebf603

magento-engcom-team added this to the Release: 2.3.3 milestone Jun 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Partial docs fixes in Newsletter module #20848

Partial docs fixes in Newsletter module #20848

Uh oh!

ghost commented Jan 31, 2019

Uh oh!

magento-engcom-team commented Jan 31, 2019

Uh oh!

orlangur Jan 31, 2019

Uh oh!

ghost Jan 31, 2019

Uh oh!

Uh oh!

Uh oh!

orlangur commented Apr 24, 2019

Uh oh!

sidolov commented Apr 29, 2019

Uh oh!

orlangur commented Apr 29, 2019

Uh oh!

p-bystritsky commented Jun 5, 2019

Uh oh!

orlangur left a comment

Uh oh!

magento-engcom-team commented Jun 5, 2019

Uh oh!

orlangur left a comment

Uh oh!

p-bystritsky commented Jun 6, 2019

Uh oh!

magento-engcom-team commented Jun 6, 2019

Uh oh!

soleksii commented Jun 7, 2019

Uh oh!

m2-assistant bot commented Jun 8, 2019

Uh oh!

Uh oh!

Partial docs fixes in Newsletter module #20848

Partial docs fixes in Newsletter module #20848

Uh oh!

Conversation

ghost commented Jan 31, 2019

Description

Contribution checklist (*)

Uh oh!

magento-engcom-team commented Jan 31, 2019

Uh oh!

orlangur Jan 31, 2019

Choose a reason for hiding this comment

Uh oh!

ghost Jan 31, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

orlangur commented Apr 24, 2019

Uh oh!

sidolov commented Apr 29, 2019

Uh oh!

orlangur commented Apr 29, 2019

Uh oh!

p-bystritsky commented Jun 5, 2019

Uh oh!

orlangur left a comment

Choose a reason for hiding this comment

Uh oh!

magento-engcom-team commented Jun 5, 2019

Uh oh!

orlangur left a comment

Choose a reason for hiding this comment

Uh oh!

p-bystritsky commented Jun 6, 2019

Uh oh!

magento-engcom-team commented Jun 6, 2019

Uh oh!

soleksii commented Jun 7, 2019

Uh oh!

m2-assistant bot commented Jun 8, 2019

Uh oh!

Uh oh!