Clean code #18124

hryvinskyi · 2018-09-18T21:39:41Z

Description

Just modify some code, and make it cleaner

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)

magento-engcom-team · 2018-09-18T21:40:17Z

Hi @hryvinskyi. 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 $VERSION instance - deploy vanilla Magento instance

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

sidolov · 2018-09-19T08:02:49Z

app/code/Magento/Catalog/Ui/Component/ColumnFactory.php

-     * @return \Magento\Ui\Component\Listing\Columns\ColumnInterface
-     */
+	/**
+	 * @param \Magento\Catalog\Api\Data\ProductAttributeInterface          $attribute


Please, remove extra spaces from doc block

Please, check comment indentation

sidolov · 2018-09-19T08:03:41Z

app/code/Magento/CatalogUrlRewrite/Model/ObjectRegistry.php

@@ -26,15 +26,19 @@ public function __construct($entities)
    }

    /**
+     * Get


Please, add a comment that describes service method

sidolov · 2018-09-19T08:03:46Z

app/code/Magento/CatalogUrlRewrite/Model/ObjectRegistry.php

    }

    /**
+     * List


Please, add a comment that describes service method

sidolov · 2018-09-19T08:04:05Z

app/code/Magento/Customer/Block/Adminhtml/Edit/Tab/View/Sales.php

+	/**
+	 * Format price by specified website
+	 *
+	 * @param float    $price


Please, remove extra spaces from doc block

Please, check comment indentation

sidolov · 2018-09-19T08:05:18Z

app/code/Magento/Fedex/Model/Carrier.php

+	 *
+	 * @return string 3-digit currency code
+	 * @throws \Magento\Framework\Exception\NoSuchEntityException
+	 */


Please, check comment indentation

sidolov · 2018-09-19T08:07:29Z

app/code/Magento/Store/Model/Store.php

+	 * @SuppressWarnings(PHPMD.CyclomaticComplexity)
+	 * @SuppressWarnings(PHPMD.NPathComplexity)
+	 * @throws \Magento\Framework\Exception\NoSuchEntityException
+	 */


Please, check comment indentation

sidolov · 2018-09-19T08:07:35Z

app/code/Magento/Store/Model/Store.php

+	 *
+	 * @return $this
+	 * @throws \Magento\Framework\Exception\LocalizedException
+	 */


Please, check comment indentation

sidolov · 2018-09-19T08:07:43Z

app/code/Magento/Store/Model/Store.php

+	 *
+	 * @return $this
+	 * @throws \Magento\Framework\Exception\NoSuchEntityException
+	 */


Please, check comment indentation

sidolov · 2018-09-19T08:07:49Z

app/code/Magento/Widget/Model/ResourceModel/Layout/Update.php

+	 *
+	 * @return string
+	 * @throws \Magento\Framework\Exception\LocalizedException
+	 */


Please, check comment indentation and remove extra spaces

sidolov · 2018-09-19T08:07:59Z

app/code/Magento/Widget/Model/ResourceModel/Layout/Update.php

+	 *
+	 * @return \Magento\Framework\DB\Select
+	 * @throws \Magento\Framework\Exception\LocalizedException
+	 */


Please, check comment indentation

hryvinskyi · 2018-09-23T12:00:22Z

@sidolov done

sidolov · 2018-09-25T08:54:11Z

app/code/Magento/ReleaseNotification/Ui/DataProvider/NotificationDataProvider.php

-     * {@inheritdoc}
+     * Get data
+     *
+     * @return mixed


Please, revert changes related to @inheritdoc. Replace {@inheritdoc} with @inheritdoc

sidolov · 2018-09-25T08:54:22Z

app/code/Magento/Sales/Model/Order/Address.php

@@ -508,207 +508,307 @@ public function getVatRequestSuccess()
    }

    /**
-     * {@inheritdoc}
+     * Set Parent ID


Please, revert changes related to @inheritdoc. Replace {@inheritdoc} with @inheritdoc

sidolov · 2018-09-25T08:54:42Z

app/code/Magento/Sales/Model/Order/Payment/Transaction.php

@@ -833,7 +833,11 @@ public function getTransactionId()
    }

    /**
-     * {@inheritdoc}
+     * Set Transaction ID


Please, revert changes related to @inheritdoc. Replace {@inheritdoc} with @inheritdoc

sidolov · 2018-09-25T08:55:02Z

app/code/Magento/Store/Model/Store.php

    }

    /**
-     * {@inheritdoc}


Please, revert changes related to @inheritdoc. Replace {@inheritdoc} with @inheritdoc

sidolov · 2018-10-04T09:07:24Z

Hi @hryvinskyi , will you continue progress with PR?

hryvinskyi · 2018-10-04T19:21:39Z

@sidolov done

sidolov · 2018-10-08T15:52:40Z

app/code/Magento/ReleaseNotification/Ui/DataProvider/NotificationDataProvider.php

@@ -133,82 +133,76 @@ public function setConfigData($config)
    }

    /**
-     * {@inheritdoc}
-     * @SuppressWarnings(PHPMD.UnusedFormalParameter)


Removing this annotation will cause static test failures because parameters are not used in the method.

sidolov · 2018-10-08T15:52:52Z

app/code/Magento/ReleaseNotification/Ui/DataProvider/NotificationDataProvider.php

     */
    public function getFieldMetaInfo($fieldSetName, $fieldName)
    {
        return [];
    }

    /**
-     * {@inheritdoc}
-     * @SuppressWarnings(PHPMD.UnusedFormalParameter)


Removing this annotation will cause static test failures because parameters are not used in the method.

sidolov · 2018-10-08T15:53:49Z

app/code/Magento/Sales/Model/Order/Address.php

     */
    public function getExtensionAttributes()
    {
        return $this->_getExtensionAttributes();
    }

    /**
-     * {@inheritdoc}
-     *
-     * @param \Magento\Sales\Api\Data\OrderAddressExtensionInterface $extensionAttributes


Please, don't remove type hints for the method because the parent method has annotation with generic interface

sidolov · 2018-10-08T15:54:13Z

app/code/Magento/Sales/Model/Order/Payment/Transaction.php

     */
    public function setIsClosed($isClosed)
    {
        return $this->setData(TransactionInterface::IS_CLOSED, $isClosed);
    }

    /**
-     * {@inheritdoc}
-     *
-     * @return \Magento\Sales\Api\Data\TransactionExtensionInterface|null


Please, don't remove type hints for the method because the parent method has annotation with generic interface

sidolov · 2018-10-08T15:54:49Z

app/code/Magento/Store/Model/Store.php

     */
    public function getScopeType()
    {
        return ScopeInterface::SCOPE_STORE;
    }

    /**
-     * {@inheritdoc}
-     * @since 100.1.0


Please, revert back the @SInCE tag

sivaschenko

Please return the deleted phpdoc annotations

sivaschenko · 2018-11-14T14:05:15Z

app/code/Magento/Sales/Model/Order/Payment/Transaction.php

@@ -992,10 +992,7 @@ public function getExtensionAttributes()
    }

    /**
-     * {@inheritdoc}
-     *
-     * @param \Magento\Sales\Api\Data\TransactionExtensionInterface $extensionAttributes


Please revert the param and return annotation for phpdoc

sivaschenko · 2018-11-14T14:05:52Z

app/code/Magento/Sales/Model/Order/Address.php

     */
    public function setVatRequestSuccess($vatRequestSuccess)
    {
        return $this->setData(OrderAddressInterface::VAT_REQUEST_SUCCESS, $vatRequestSuccess);
    }

    /**
-     * {@inheritdoc}
-     *
-     * @return \Magento\Sales\Api\Data\OrderAddressExtensionInterface|null


Please revert the return annotation for phpdoc

sivaschenko · 2018-11-14T14:08:27Z

Hi, @hryvinskyi , please resolve the code review notes, fix the Travis build (static tests fails) and resolve the conflicts on your branch, so that we can process your pull request!

magento-engcom-team · 2018-11-26T12:13:29Z

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

magento-engcom-team · 2018-12-12T20:43:05Z

Hi @hryvinskyi. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

Clean code

1e4dd7d

magento-engcom-team added Component: multiple Release Line: 2.3 labels Sep 18, 2018

sidolov requested changes Sep 19, 2018

View reviewed changes

sidolov added the Progress: needs update label Sep 19, 2018

sidolov self-assigned this Sep 19, 2018

hryvinskyi added 3 commits September 20, 2018 22:00

Update Tests

46fdf6d

fix tests

1b5f7c5

fix tests

7aeb3be

sidolov requested changes Sep 25, 2018

View reviewed changes

Update Comments

cdd19a6

Fix pr

9dc3345

sidolov requested changes Oct 8, 2018

View reviewed changes

update comments

1ce85bb

sivaschenko requested changes Nov 14, 2018

View reviewed changes

sivaschenko self-assigned this Nov 21, 2018

Merge branch '2.3-develop' into PR-port-1

ffc7593

sivaschenko approved these changes Nov 26, 2018

View reviewed changes

magento-engcom-team added this to the Release: 2.3.1 milestone Nov 26, 2018

magento-engcom-team added Progress: accept and removed Progress: needs update labels Nov 26, 2018

ENGCOM-3567: Static tests fixed.

09c9d74

magento-engcom-team merged commit 09c9d74 into magento:2.3-develop Dec 12, 2018

magento-engcom-team added the Cleanup label Dec 12, 2018

magento-engcom-team pushed a commit that referenced this pull request Dec 12, 2018

ENGCOM-3567: Clean code #18124

d2a69f3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean code #18124

Clean code #18124

hryvinskyi commented Sep 18, 2018 •

edited

Loading

magento-engcom-team commented Sep 18, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

sidolov Sep 19, 2018

hryvinskyi commented Sep 23, 2018

sidolov Sep 25, 2018

sidolov Sep 25, 2018

sidolov Sep 25, 2018

sidolov Sep 25, 2018

sidolov commented Oct 4, 2018

hryvinskyi commented Oct 4, 2018

sidolov Oct 8, 2018

sidolov Oct 8, 2018

sidolov Oct 8, 2018

sidolov Oct 8, 2018

sidolov Oct 8, 2018

sivaschenko left a comment

sivaschenko Nov 14, 2018

sivaschenko Nov 14, 2018

sivaschenko commented Nov 14, 2018

magento-engcom-team commented Nov 26, 2018

magento-engcom-team commented Dec 12, 2018

@@ @@ -26,15 +26,19 @@ public function __construct($entities) @@
                   }
                   /**
+                   * Get

                   }
                   /**
+                   * List

Clean code #18124

Clean code #18124

Conversation

hryvinskyi commented Sep 18, 2018 • edited Loading

Description

Contribution checklist

magento-engcom-team commented Sep 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hryvinskyi commented Sep 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sidolov commented Oct 4, 2018

hryvinskyi commented Oct 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sivaschenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sivaschenko commented Nov 14, 2018

magento-engcom-team commented Nov 26, 2018

magento-engcom-team commented Dec 12, 2018

hryvinskyi commented Sep 18, 2018 •

edited

Loading