Skip to content

PHP Codesniffer pass#78

Open
pbritka wants to merge 4 commits intoWeareJH:masterfrom
pbritka:phpcs
Open

PHP Codesniffer pass#78
pbritka wants to merge 4 commits intoWeareJH:masterfrom
pbritka:phpcs

Conversation

@pbritka
Copy link

@pbritka pbritka commented Sep 16, 2025

I run php codesniffer and it couldn't pass the Magento2 technical review. I added proper output escaping, replaced md5 with sha256 and fixed

  • previously removed preference for Symfony\Component\Console\Output\OutputInterface which made it unable to run the import from cli
  • fixed action that downloaded csv files from admin import configuration detail

The code still doesn't pass the technical review as it doesn't use db_schema.xml, but I saw another pull request that fixes this problem

Peter Britka added 2 commits September 3, 2025 22:37
… back preference for Symfony\Component\Console\Output\OutputInterface
private function hashRecord(Record $record): string
{
return md5(json_encode($record->asArray(), JSON_THROW_ON_ERROR));
return hash('sha256', json_encode($record->asArray(), JSON_THROW_ON_ERROR));
Copy link
Contributor

Choose a reason for hiding this comment

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

is this backward compatible?

Copy link
Author

Choose a reason for hiding this comment

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

I see no reason why it shouldn't be. It's a standard PHP function

Copy link
Author

Choose a reason for hiding this comment

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

But the preference for Symfony\Component\Console\Output\OutputInterface shouldn't be there. It was resolved in another issue. I'll look at this issue later and will fix the commits

Copy link
Contributor

Choose a reason for hiding this comment

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

Preference issue is solved on main - see your issue.

I ask if it is compatible because someone already using import module wont be able to compare old hashes with new ones (if old ones are kept in db for example). I have to check what that function does to make sure we are not breaking anything with it.

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.

2 participants