Skip to content

fileinfo: Deprecate finfo_close() #18396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TimWolla
Copy link
Member

Does this need an RFC or can we do this by simple agreement? Requesting reviews from Girgias and the RMs 😄


This is for consistency with other *_close() functions that have become obsolete when migrating from resources to objects.

@DanielEScherzer
Copy link
Member

@TimWolla TimWolla added the RFC label Apr 22, 2025
@TimWolla
Copy link
Member Author

Thanks, I've linked this PR in the RFC as the implementation.

@Girgias
Copy link
Member

Girgias commented Apr 23, 2025

I think merging the test changes already is fine.

But having it in the bulk RFC is probably the "proper" way to do it, even if I would agree just deprecating it should be fine.

@TimWolla
Copy link
Member Author

I think merging the test changes already is fine.

#18405.

This is for consistency with other `*_close()` functions that have become
obsolete when migrating from resources to objects.
@TimWolla TimWolla force-pushed the finfo-close-deprecate branch from 3ebec92 to bad3cef Compare April 23, 2025 16:58
@@ -92,6 +92,7 @@ public function set_flags(int $flags): true {}
/** @refcount 1 */
function finfo_open(int $flags = FILEINFO_NONE, ?string $magic_database = null): finfo|false {}

#[\Deprecated(since: '8.5', message: 'as finfo objects are freed automatically')]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[\Deprecated(since: '8.5', message: 'as finfo objects are freed automatically')]
#[\Deprecated(since: 'PHP 8.5', message: 'as finfo objects are freed automatically')]

Isn't it better to be explicit about the version? There may be case that the plain version number may be confused with other software, i.e. MySQL has close version number.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a discussion for another place. The “bare version” format is what is currently used by PHP core. See: 29f98e7

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.

4 participants