Skip to content

Drop support for statically calling these methods#4680

Closed
cmb69 wants to merge 1 commit into
php:masterfrom
cmb69:xmlreader-static
Closed

Drop support for statically calling these methods#4680
cmb69 wants to merge 1 commit into
php:masterfrom
cmb69:xmlreader-static

Conversation

@cmb69

@cmb69 cmb69 commented Sep 4, 2019

Copy link
Copy Markdown
Member

Calling non-static methods statically is deprecated as of PHP 7.0.0,
and forbidden as of PHP 8.0.0, anyway.

Calling non-static methods statically is deprecated as of PHP 7.0.0,
and forbidden as of PHP 8.0.0, anyway.
@cmb69

cmb69 commented Sep 4, 2019

Copy link
Copy Markdown
Member Author

The somewhat cleaner solution would be to mark these methods as static, and remove support for calling them non-statically. That would constitute a more serious BC break, though.

@KalleZ

KalleZ commented Sep 4, 2019

Copy link
Copy Markdown
Member

@cmb69 I think marking them as static is a better solution, given the major version bump I don't see any issue with that. Thoughts?

@cmb69

cmb69 commented Sep 4, 2019

Copy link
Copy Markdown
Member Author

@KalleZ, the only drawback I see with this solution, is that we told users to call them non-statically since PHP 7. Of course, still cleaner to have them as factory methods (maybe we also could simplify the code a bit).

@nikic

nikic commented Sep 4, 2019

Copy link
Copy Markdown
Member

It would be possible to mark these as static, but still support instance calls in the implementation for BC purposes (possibly with a deprecation). While we don't allow this in userland, we can do it internally. And if it's with eventual migration in mind, maybe that's okay?

@KalleZ

KalleZ commented Sep 4, 2019

Copy link
Copy Markdown
Member

@nikic sounds reasonable to me, to at least give userland a migration period and ease 8.0 adoption

@cmb69

cmb69 commented Sep 4, 2019

Copy link
Copy Markdown
Member Author

I had a closer look, and if we really want to switch to factory methods, we also should drop the ::close() method; otherwise you could close the reader without any chance to open it again, but still would have the then useless XMLReader instance.

@KalleZ

KalleZ commented Sep 4, 2019

Copy link
Copy Markdown
Member

@cmb69 That makes good sense too, +1 from me on deprecating those if we go down this route and pointing users to use unset() on them or similar like any other object.

@cmb69

cmb69 commented Oct 11, 2019

Copy link
Copy Markdown
Member Author

Superseded by PR #4816; closing.

@cmb69 cmb69 closed this Oct 11, 2019
@cmb69 cmb69 deleted the xmlreader-static branch October 11, 2019 14:51
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