WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@yashgit891
Copy link
Contributor

Note :- Please follow the below points while attaching test cases document link below:

- If label Tested is added then test cases document URL is mandatory.

- Link added should be a valid URL and accessible throughout the org.

- If the branch name contains hotfix / revert by default the BVT workflow check will pass.

Test Case Document URL
Please paste test case document link here....

@yashgit891 yashgit891 requested a review from ramth05 February 13, 2024 10:37
Copy link

@lbajsarowicz lbajsarowicz left a comment

Choose a reason for hiding this comment

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

⚠️ Quite a lot of issues, mostly with code maintainability and reliability.

Choose a reason for hiding this comment

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

Please make sure your code follows PSR standard for PHP.
... and even more important: It follows Magento Coding Standards.

2.7. All non-public properties and methods SHOULD be private.

https://developer.adobe.com/commerce/php/coding-standards/technical-guidelines/

Comment on lines +7 to +10
/**
* @var \Razorpay\Magento\Model\Config
*/
protected $config;

Choose a reason for hiding this comment

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

Suggested change
/**
* @var \Razorpay\Magento\Model\Config
*/
protected $config;
/**
* @var \Razorpay\Magento\Model\Config
*/
private $config;

Comment on lines +12 to +15
/**
* @var \Psr\Log\LoggerInterface
*/
protected $logger;

Choose a reason for hiding this comment

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

Suggested change
/**
* @var \Psr\Log\LoggerInterface
*/
protected $logger;
/**
* @var \Psr\Log\LoggerInterface
*/
private $logger;

Comment on lines +19 to +28
public function __construct(
\Razorpay\Magento\Model\Config $config,
\Psr\Log\LoggerInterface $logger
)
{
$this->config = $config;
$this->logger = $logger;

$this->isDebugModeEnabled = $this->config->isDebugModeEnabled();
}

Choose a reason for hiding this comment

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

  1. Import classes
  2. Lint your code

$this->logger->info($message);
}
}
} No newline at end of file

Choose a reason for hiding this comment

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

We are working in the *nix environment, make sure to leave empty line at the EOF (PSR)

{
foreach ($orderLink as $orderData)
{
$this->debug->log("Cronjob: Magento Order Id = " . $orderData['order_id'] . " picked for updation");

Choose a reason for hiding this comment

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

Please acknowledge PSR-3 standard (https://www.php-fig.org/psr/psr-3/) for LoggerInterface and use the 2nd argument -- $context. Example:

Suggested change
$this->debug->log("Cronjob: Magento Order Id = " . $orderData['order_id'] . " picked for updation");
$this->logger->debug("Cronjob: Magento Order picked for update", ['order_id' => $orderData['order_id']]);

PS: What is "updation"?

/**
* @var \Razorpay\Magento\Model\Util\DebugMode
*/
protected $debug;

Choose a reason for hiding this comment

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

2.7. All non-public properties and methods SHOULD be private.

https://developer.adobe.com/commerce/php/coding-standards/technical-guidelines/

Suggested change
protected $debug;
private $debug;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants