Skip to content

Promote warnings to exceptions in ext/curl - #5963

Closed
kocsismate wants to merge 1 commit into
php:masterfrom
kocsismate:curl-warning
Closed

kocsismate wants to merge 1 commit into
php:masterfrom
kocsismate:curl-warning

Conversation

@kocsismate

@kocsismate kocsismate commented Aug 9, 2020 •

Copy link
Copy Markdown
Member

It should be checked if there is any other warnings in ext/curl that should be promoted

@kocsismate kocsismate left a comment •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Test changes are WIP

Comment thread ext/curl/interface.c Outdated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe it would be worth to introduce a new function that adds the current function/method name to the beginning. It might be useful if we start adding OO interfaces.

Comment thread ext/curl/interface.c Outdated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

is writing cURL ok?

@kocsismate
kocsismate force-pushed the curl-warning branch 5 times, most recently from 8d56a60 to b6e248a Compare August 10, 2020 21:35
Comment thread ext/curl/interface.c Resolved
@php-pulls php-pulls closed this in 517c993 Aug 14, 2020
lacatoire pushed a commit to php/doc-en that referenced this pull request Oct 9, 2026
* curl_setopt(_array): document ValueError for invalid options

Add Errors sections and PHP 8.0.0 changelog entries noting that
invalid cURL options now throw ValueError instead of emitting
E_WARNING (php/php-src#5963 / PHP 8 internal function exceptions).

Closes #3119

* Apply review suggestions; use simpara in Errors sections
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.

2 participants