Revert "Deprecation errors are not suppressed anymore" #3102

[sreichel]

Description (*)

Please revert #3102!

With php 81. i cant neither use main because of #3175 nor v19 with enabled dev-mode because of deprecation errors.

Related Pull Requests

1. See Revert "Deprecation errors are not suppressed anymore (#3102)" #3176

Manual testing scenarios (*)

1. use php8.1
2. enable developer mode
3. run into errors
4. ....

Questions or comments

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All automated tests passed successfully (all builds are green)
• Add yourself to contributors list

[addison74]

I faced this problem. the application will not run, the modification must be done manually.

However, #3102 brought advantages that we cannot deny. That's how I discovered a lot of errors/warnings in 8.2 that we didn't know about and which are mostly fixed. Unfortunately, I am the only one who approved those PRs and we've been in this situation for a long time without incorporating them into the OpenMage code faster.

[elidrissidev]

FYI there are 2 ways to prevent errors (deprecations or otherwise) from being thrown and blocking the page:

1. You just disable Developer Mode and all errors will be logged instead of thrown.
2. Update error_reporting directive in your php.ini (globally or via .user.ini) to only include the errors you're interested in.

Do these options not cover your case?

[fballiano]

cleaning an orphan eav attribute should be a matter of seconds.

[sreichel]

@elidrissidev disable developer mode is not an option. What was wrong with setting env DEV_PHP_STRICT=1 to optionally show deprecation warnings? (#2882)

[fballiano]

What was wrong was already explain in the other pr.

[sreichel]

This?

Cause it's better that people start to see this logs and fix their deprecation errors

Please let the users decide to show/hide deprecaction errors. With php8.1 there are still a lot, but i dont want to deal with them every time.

[fballiano]

Exactly, nobody wants to deal with these prs every time

[fballiano]

While we are trying to fix the few remainings of the 8.1 warnings

[sreichel]

@fballiano the env var was added for those who activly work on php81+ support - without changing behavoir for others.

I cant work on other things while running 8.1 .... :/

[addison74]

The PR description is not exactly the right one. This is not about deleting an orphaned attribute after removing an extension. That error is confirmed and we are still waiting for a solution from the one who made the recent changes in EAV.

The question that arises is why do we need another environment variable DEV_PHP_STRICT when we already have MAGE_IS_DEVELOPER_MODE? From the explanation given by @sreichel it appears that it is useful especially for the PHP8.1 version. The first variable D_P_S covers parts that the second variable M_I_D_M does not, practically they can work in parallel.

[sreichel]

The question that arises is why do we need another environment variable DEV_PHP_STRICT when we already have MAGE_IS_DEVELOPER_MODE?

DEV-mode controlls different things, DEV_PHP_STRICT was just to show deprecation errors optionally. (escpacially for 8.1+)

Updated PR to optionally hide them.

[fballiano]

You could download this pr and apply it as a composer patch to your project

[addison74]

In such situations, a democratic vote would be the best solution. As far as I am concerned, I have nothing against this PR to be included in OpenMage. Actually it was before in the code but was removed. There were other similar cases when developers wanted to solve their particular situations and proposed PRs in this regard and we approved them. However, Sven is an important contributor to this project and his proposals over time have found their rightful place, beyond some personal disputes. Let's not easily forget the integration of ZF1-Future, PHPStan and many others.

I hope we don't end up proposing patches to everyone who wants basic features. For example, OpenMage v20 needs formkeys for the Contact and Newletter forms (for Checkout it already has) but the same thing was recommended to me, "create your own patch!". In this way we could only create PRs that remain unapproved and everyone chooses the ones they need. Am I right?

[sreichel]

You could download this pr and apply it as a composer patch to your project

You can only patch files in vendor, not in project files ... no.

[addison74]

In the case of ZF1-Future, we outsourced the code by linking it to the developer's repository and it ended up in the vendor directory after which we applied some patches, Composer making the work possible.

This page that was approved recently https://www.openmage.org/2023/05/01/customize-your-openmage.html, it provides a solution for those who do not see their ideas accepted in OpenMage. Now it comes the question, in order to patch the OpenMage code, do we need in vendor an openmage/magento-lts directory? I don't think this aspect has been analyzed yet.

[fballiano]

you can do it with zero problems:
https://github.com/fballiano/mcommerce/blob/main/composer.json

[fballiano]

at this point it would be better to mage developer mode = error reporting

[fballiano]

I hope we don't end up proposing patches to everyone who wants basic features.

was this ever done? never. so, in this case, it was done for a reason.

[sreichel]

you can do it with zero problems:

Locally i work with git pull and have no openmage/magento-lts dependency.

[sreichel]

Can we just restore a working behavoir? (At least for v19)

• deprecation warnings were hidden all the time ... (b/c they do not harm)
• a ENV var was added to show/hide them
• everything got removed and everyone faces deprecation warnings for php81

I REALLY dont understand what we are discussing bout.

Actually the best way to work arround it, is not to use php81 at all.

[Flyingmana]

is there a problem with the mentioned "error_reporting" approach`? Do we overwrite the ini setting in OpenMage? Because then we should make this configurable for Dev Mode, as people might want different levels, sometimes without error reporting, sometimes even without notices.
Personally I did always hardcode changed error reporting somewhere 🤔

[sreichel]

as people might want different levels, sometimes without error reporting, sometimes even without notices.

Thats it, This change killed the possibilty to hide/show deprecation warnings ...

[Flyingmana]

So I tested it out, and I could confirm the error_reporting is not overwritten from the dev mode, so setting in the php.ini error_reporting = 24575 (which is E_ALL & ~E_DEPRECATED) prevents your reported issue.
Therefor I vote for closing this PR

[sreichel]

PR was w/o adjusting php.ini ...

[sreichel]

Thank for wasting my time.