Skip to content

Conversation

@oshmyheliuk
Copy link
Contributor

@oshmyheliuk oshmyheliuk commented Jul 9, 2019

@magento-cicd2
Copy link

magento-cicd2 commented Jul 9, 2019

CLA assistant check
All committers have signed the CLA.

@oshmyheliuk oshmyheliuk added Release: 1.0.1 MCC Release Progress: review PR/issue status labels Jul 9, 2019
public function __construct(
StoreManagerInterface $storeManager
) {
parent::__construct();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parent constructor should go after properties assignment after a newline

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Is this Magento code standard?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most Magento code use parent after

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not most, we don't have such rule.

protected function configure()
{
$this->setName('config:show:default-url')
->setDescription(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No newlines needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

$output->write($store->getBaseUrl(UrlInterface::URL_TYPE_LINK, $store->isUrlSecure()));

return Cli::RETURN_SUCCESS;
} catch (\Exception $e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This try-catch does not provide any value, can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

/**
* @param StoreManagerInterface $storeManager
*/
public function __construct(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No newlines needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@BaDos BaDos merged commit 5499165 into master Jul 19, 2019
@BaDos BaDos deleted the MAGECLOUD-3866 branch July 19, 2019 14:35
@YPyltiai YPyltiai added Progress: approved PR/issue status and removed Progress: review PR/issue status labels Jul 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Progress: approved PR/issue status Release: 1.0.1 MCC Release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants