Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:77454 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 31954 invoked from network); 22 Sep 2014 11:29:46 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 22 Sep 2014 11:29:46 -0000 Authentication-Results: pb1.pair.com smtp.mail=derick@php.net; spf=unknown; sender-id=unknown Authentication-Results: pb1.pair.com header.from=derick@php.net; sender-id=unknown Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 82.113.146.227 as permitted sender) X-PHP-List-Original-Sender: derick@php.net X-Host-Fingerprint: 82.113.146.227 xdebug.org Linux 2.6 Received: from [82.113.146.227] ([82.113.146.227:55436] helo=xdebug.org) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id D0/00-31799-82800245 for ; Mon, 22 Sep 2014 07:29:45 -0400 Received: from localhost (localhost [IPv6:::1]) by xdebug.org (Postfix) with ESMTPS id EF1C4115404; Mon, 22 Sep 2014 12:29:41 +0100 (BST) Date: Mon, 22 Sep 2014 12:29:41 +0100 (BST) X-X-Sender: derick@whisky.home.derickrethans.nl To: Florian Margaine cc: PHP Developers Mailing List In-Reply-To: Message-ID: References: User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Subject: Re: [PHP-DEV] ext/date arguments handling From: derick@php.net (Derick Rethans) Hi Florian, On Sun, 21 Sep 2014, Florian Margaine wrote: > I specifically mean to ask @derick about this issue I'm having, since > he is the maintainer of ext/date. > > I wrote this pull request for the DateTimeZone::getOffset method to > accept a DateTimeInterface instead of a DateTime object: > https://github.com/php/php-src/pull/831 > > Instead of using ZEND_ARG_INFO, I use ZEND_ARG_OBJ_INFO rather than > relying on zpp only. It makes the code consistent with the type > hinting errors that should arise, and also gives a correct reflection. > > However, the rest of the code in this extension uses ZEND_ARG_INFO, > only throwing warnings when the arguments are not of the right kind. > > Should I use ZEND_ARG_INFO and rely on zpp's warning, or should I use > ZEND_ARG_OBJ_INFO, and eventually translate the other methods to use > it too? This'd be out of this PR of course, but it makes sense to > streamline the methods of ext/date. I think it should use ZEND_ARG_OBJ_INFO. However, I am not sure whether we can do that in 5.x because of BC reasons. It's these tiny mistakes that are the larger BC problems. So I would suggest that you make a PR for 5.6 without ZEND_ARG_OBJ_INFO, and one for 7 with ZEND_ARG_OBJ_INFO. I think the patch looks mostly good too. I would recommend you squash the commits into a single commit though. cheers, Derick