Skip to content

ext/spl: Remove spl_engine.h header - #14418

Merged
Girgias merged 2 commits into
php:masterfrom
Girgias:spl-engine-remove
Jun 8, 2024
Merged

ext/spl: Remove spl_engine.h header#14418
Girgias merged 2 commits into
php:masterfrom
Girgias:spl-engine-remove

Conversation

@Girgias

Copy link
Copy Markdown
Member

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

@ndossche

Copy link
Copy Markdown
Member

To be honest, I found the old code clearer.

@TimWolla

Copy link
Copy Markdown
Member

Somewhat related #14254.

@Girgias

Copy link
Copy Markdown
MemberAuthor

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)

@ndossche

Copy link
Copy Markdown
Member

Having a better API that instantiates would be very welcome.

@Girgias
Girgiasforce-pushed the spl-engine-remove branch from e643ee8 to 3fd2257CompareJune 6, 2024 22:44
@Girgias
Girgias marked this pull request as ready for review June 6, 2024 23:20
@Girgias

Copy link
Copy Markdown
MemberAuthor

Improved the PR after the merging of #14440

@Girgias
Girgias requested a review from ndosscheJune 7, 2024 14:51
Comment threadext/spl/spl_directory.c Outdated
Comment threadext/spl/spl_directory.c Outdated
Comment threadext/spl/spl_iterators.c Outdated
Comment threadext/spl/spl_iterators.c Outdated
Comment threadext/spl/spl_iterators.c Outdated
Comment threadext/spl/spl_iterators.c Outdated
Comment threadext/spl/spl_iterators.c Outdated
Comment threadext/spl/spl_iterators.c Outdated
Comment threadext/spl/spl_iterators.c Outdated
Comment threadext/spl/spl_iterators.c Outdated
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
Girgiasforce-pushed the spl-engine-remove branch from 3fd2257 to 573a363CompareJune 8, 2024 16:27
@Girgias
Girgias merged commit a5cacba into php:masterJun 8, 2024
@Girgias
Girgias deleted the spl-engine-remove branch June 8, 2024 22:46
Sign up for freeto 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

@Girgias@ndossche@TimWolla