Skip to content

[PHP 8] Resources object upgrade - remove 2 rules as the upgrade require wholesome manual work#4442

Merged
TomasVotruba merged 2 commits intomainfrom
tv-less-resources
Jul 8, 2023
Merged

[PHP 8] Resources object upgrade - remove 2 rules as the upgrade require wholesome manual work#4442
TomasVotruba merged 2 commits intomainfrom
tv-less-resources

Conversation

@TomasVotruba
Copy link
Copy Markdown
Member

@TomasVotruba TomasVotruba commented Jul 8, 2023

This upgrade depends on exact new feature of resources, see:

They should be handled with resource or resource/object or object in mind. In latter use the object methods etc. This needs manual work with careful human touch :)

@TomasVotruba TomasVotruba changed the title tv less resources [PHP 8] Resources object upgrade - remove 2 rules as the upgrade require wholesome manual work Jul 8, 2023
@TomasVotruba TomasVotruba force-pushed the tv-less-resources branch 2 times, most recently from b223133 to 057f9c9 Compare July 8, 2023 22:42
@TomasVotruba TomasVotruba merged commit b9ace5f into main Jul 8, 2023
@TomasVotruba TomasVotruba deleted the tv-less-resources branch July 8, 2023 22:45
@kocsismate
Copy link
Copy Markdown

Hi @TomasVotruba,

I'm not really sure why this rule was removed, even by reading the description. Could you please tell me what kind of manual work is needed? As far as I remember, only the widely known is_resource($r) calls to $r !== false changes are needed to ensure BC apart from some edge-cases like this as well as the removal of some deprecated functions (e.g. shmop_close()).

@TomasVotruba
Copy link
Copy Markdown
Member Author

TomasVotruba commented Nov 5, 2023

Hi, it depends on the code base. The risky place here are strict type declarations. We applied this rule on one project, but it completely broke the rest of the code, as different types were not used there. Thus it's required to handle this manually and with caution.

Despite that, feel free to copy paste this rule in own custom set.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants