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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants