Skip to content

Conversation

loginesta
Copy link
Contributor

@loginesta loginesta commented Aug 25, 2021

This sniffer looks for occurrences of obsolete nodes: param, instance, array, item[@key] and value.
Returns a warning for each occurrence found.
https://jira.corp.magento.com/browse/AC-660

- DiConfigTest: Test for obsolete nodes in di.xml
@sivaschenko sivaschenko changed the title AC-660: Create phpcs static check for DiConfigTest Create phpcs static check for DiConfigTest Aug 25, 2021
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @loginesta ! Can you please verify the test functionality running it against the Magento code with changed di.xml files?

@loginesta loginesta marked this pull request as ready for review August 27, 2021 14:31
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @loginesta ! Please see my comments

{
private const WARNING_CODE = 'FoundObsoleteAttribute';

private $xpathObsoleteElems = [
Copy link
Member

@sivaschenko sivaschenko Aug 27, 2021

Choose a reason for hiding this comment

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

I would renamve this property as it's not xpath anymore, also it would be good to add a phpdoc

Suggested change
private $xpathObsoleteElems = [
private $obsoleteDiNodes = [
{
$lineContent = $phpcsFile->getTokensAsString($stackPtr, 1);

foreach ($this->xpathObsoleteElems as $elem => $message) {
Copy link
Member

Choose a reason for hiding this comment

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

It's a good pratice to use whole words for variables naming

Suggested change
foreach ($this->xpathObsoleteElems as $elem => $message) {
foreach ($this->xpathObsoleteElems as $element => $message) {
- Refactor: Rename variables and add phpdoc
@sivaschenko
Copy link
Member

@magento import pr to magento-commerce/magento-coding-standard

@magento-engcom-team
Copy link
Contributor

@sivaschenko the pull request successfully imported.

@magento-devops-reposync-svc magento-devops-reposync-svc merged commit 20b1378 into magento:develop Aug 30, 2021
@loginesta loginesta deleted the AC-660 branch September 1, 2021 08:27
magento-devops-reposync-svc pushed a commit that referenced this pull request Aug 9, 2023
…o-coding-standard-453 [Imported] Update supported PHP versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

5 participants