Hi Davey, > -----Original Message----- > From: [email protected] [mailto:[email protected]] On Behalf Of Davey > Shafik > Sent: Thursday, April 28, 2016 11:30 PM > To: Pierrick Charron <[email protected]> > Cc: Anatol Belski <[email protected]>; PHP internals > <[email protected]>; [email protected] > Subject: Re: [PHP-DEV] Re: ext/curl update > > I seem to have created some confusion here: > > The reason _my_ patch for Server Push isn't merged is tests for it were > requested and are blocking it. I'm not saying tests for these constants > should be > added. > Yes, I was also reading this, that the "tests" was related to your PR. Cleared out now, anyway.
Regards anatol > - Davey > > On Wed, Apr 27, 2016 at 4:15 PM, Pierrick Charron <[email protected]> wrote: > > > Sorry for the 2 mails but I forgot to give you the URL : > > > > https://github.com/php/php-src/pull/1890/files > > > > On 27 April 2016 at 19:14, Pierrick Charron <[email protected]> wrote: > > > >> Hi Anatol, > >> > >> I created a new patch from the one first published but this time this > >> one target 7.0 and only expose new constants to that do not require > >> any logic on the extension side. > >> These constants are just exposed if they are available in the version > >> installed and are bridge in the curl_setop function. > >> > >> If that's OK I'll commit this on 7.0 and merge it also on master. > >> Then I'll work on adding new things that require logic and clean > >> those for 7.1 and add tests if possible. > >> > >> Regards > >> Pierrick > >> > >> On 27 April 2016 at 12:55, Anatol Belski <[email protected]> wrote: > >> > >>> > >>> > >>> > -----Original Message----- > >>> > From: [email protected] [mailto:[email protected]] On Behalf > >>> > Of > >>> Pierrick > >>> > Charron > >>> > Sent: Wednesday, April 27, 2016 6:20 PM > >>> > To: Anatol Belski <[email protected]> > >>> > Cc: Davey Shafik <[email protected]>; PHP internals < > >>> [email protected]>; > >>> > [email protected] > >>> > Subject: Re: [PHP-DEV] Re: ext/curl update > >>> > > >>> > Yep I'll check if I can add some test that could be easy to > >>> > implement > >>> using > >>> > curl_easy_getinfo or using the php local server. Otherwise not > >>> > sure we > >>> could > >>> > easily compile PHP with all those libcurl versions... > >>> > > >>> > > >>> > On 27 April 2016 at 11:37, Anatol Belski <[email protected] > >>> > <mailto:[email protected]> > wrote: > >>> > > >>> > > >>> > Hi, > >>> > > >>> > > -----Original Message----- > >>> > > From: [email protected] <mailto:[email protected]> > >>> > [mailto:[email protected] <mailto:[email protected]> ] On > >>> Behalf Of > >>> > Pierrick > >>> > > Charron > >>> > > Sent: Wednesday, April 27, 2016 2:20 PM > >>> > > To: Anatol Belski <[email protected] > >>> > <mailto:[email protected]> > > >>> > > Cc: Davey Shafik <[email protected] <mailto:[email protected]> >; > >>> > PHP internals <[email protected] <mailto:[email protected]> > >>> > >; > >>> > > [email protected] <mailto:[email protected]> > >>> > > Subject: Re: [PHP-DEV] Re: ext/curl update > >>> > > > >>> > > I agree, but I don't really now how I could test those > >>> > things > >>> since they > >>> > almost all > >>> > > of the time only affect how libcurl will handle the > >>> request/cache and > >>> > we have no > >>> > > way to retrieve options like curl_easy_getopt or something > >>> similar. > >>> > > > >>> > > On 27 April 2016 at 02:46, Anatol Belski < > >>> [email protected] > >>> > <mailto:[email protected]> > >>> > > <mailto:[email protected] > >>> > <mailto:[email protected]> > >>> > > > >>> > wrote: > >>> > > > >>> > > > >>> > > Hi, > >>> > > > >>> > > > -----Original Message----- > >>> > > > From: [email protected] <mailto:[email protected]> > >>> > <mailto:[email protected] <mailto:[email protected]> > > >>> > > [mailto:[email protected] <mailto:[email protected]> > >>> > <mailto:[email protected] <mailto:[email protected]> > ] On > >>> > Behalf > >>> Of > >>> > > Davey > >>> > > > Shafik > >>> > > > Sent: Sunday, April 24, 2016 2:25 AM > >>> > > > To: Pierrick Charron <[email protected] > >>> > <mailto:[email protected]> <mailto:[email protected] > >>> > <mailto:[email protected]> > > > >>> > > > Cc: PHP internals <[email protected] > >>> > <mailto:[email protected]> > >>> > > >>> > > <mailto:[email protected] <mailto: > >>> [email protected]> > >; > >>> > [email protected] <mailto:[email protected]> <mailto:[email protected] > >>> > <mailto:[email protected]> > > >>> > > > Subject: [PHP-DEV] Re: ext/curl update > >>> > > > > >>> > > > Hi Pierrick, > >>> > > > > >>> > > > This should be in master for 7.1, alongside my RFC'ed > >>> patch for > >>> > server > >>> > > push > >>> > > > support. > >>> > > > > >>> > > > You emailed me directly about the aforementioned patch > >>> so I'll > >>> > just > >>> > > respond > >>> > > > here as it's relevant: > >>> > > > > >>> > > > The patch should hit in 7.1 but it has been requested > >>> that tests be > >>> > > added — and > >>> > > > we can't add tests with a server push supporting > >>> HTTP/2 server > >>> > against > >>> > > which to > >>> > > > push. > >>> > > > > >>> > > As from the patch, many constants have nothing to do > >>> with HTTP/2 > >>> > > implementation and add just name/value without any further > >>> logic. If > >>> > there were > >>> > > a reduced patch with only such cases, it would be > >>> > acceptable > >>> for 7.0 > >>> > as well and > >>> > > there were probably no collisions expected. What do you think? > >>> > > > >>> > > >>> > So far I understood tests are exactly about HTTP2. Not sure > >>> > how > >>> you > >>> > would tests all the constants present in libcurl. Would need to > >>> rebuild with a > >>> > dozen libcurl versions, but the documentation and compile time > >>> > version > >>> check > >>> > are already reliable things. > >>> > > >>> But if you can fish out only the cases with name/value which don't > >>> interfere with the HTTP2 work, so it's fine to add. OFC it were > >>> absurd to recompile with all libcurl versions :) especially as an > >>> excellent documentation to every option is presend on the cURL side. > >>> If that's only an option that say affects the curl behavior and > >>> don't require any extra handling, I don't think it is critical. On > >>> the other hand, if an option requires some pre/post handling an thus > >>> some extra implementation - then OFC it should be urgently suggested to > have a good test. > >>> > >>> Thanks > >>> > >>> Anatol > >>> > >>> > >> > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
