Skip to content

ext/spl: Remove spl_engine.h header #14418

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

Merged
merged 2 commits into from
Jun 8, 2024
Merged

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jun 1, 2024

This doesn't really make the calling code clearer and it is barely used on top of that.

@nielsdos
Copy link
Member

nielsdos commented Jun 1, 2024

To be honest, I found the old code clearer.

@TimWolla
Copy link
Member

TimWolla commented Jun 1, 2024

Somewhat related #14254.

@Girgias
Copy link
Member Author

Girgias commented Jun 2, 2024

To be honest, I found the old code clearer.

It might make sense to have a better API provided by Zend to instantiate an object and run the constructor (e.g. object_init_with_constructor(zval *arg, zend_class_entry *class_type, uint32_t param_count, zval *params)) but personally I find the names in that header rather confusing about what it is doing.

Moreover, I don't really like having other extensions using SPL headers (knowing that with the current state of iterators some extension will include them anyway)

@nielsdos
Copy link
Member

nielsdos commented Jun 2, 2024

Having a better API that instantiates would be very welcome.

@Girgias Girgias force-pushed the spl-engine-remove branch from e643ee8 to 3fd2257 Compare June 6, 2024 22:44
@Girgias Girgias marked this pull request as ready for review June 6, 2024 23:20
@Girgias
Copy link
Member Author

Girgias commented Jun 6, 2024

Improved the PR after the merging of #14440

@Girgias Girgias requested a review from nielsdos June 7, 2024 14:51
Girgias added 2 commits June 8, 2024 17:20
This doesn't really make the calling code clearer and it is barely used on top of that.
@Girgias Girgias force-pushed the spl-engine-remove branch from 3fd2257 to 573a363 Compare June 8, 2024 16:27
@Girgias Girgias merged commit a5cacba into php:master Jun 8, 2024
11 checks passed
@Girgias Girgias deleted the spl-engine-remove branch June 8, 2024 22:46
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.

3 participants