Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:1567 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 21647 invoked from network); 16 May 2003 13:46:19 -0000 Received: from unknown (HELO jdi.jdimedia.nl) (212.204.192.51) by pb1.pair.com with SMTP; 16 May 2003 13:46:19 -0000 Received: from jdi.jdimedia.nl (jdi.jdimedia.nl [212.204.192.51]) by jdi.jdimedia.nl (8.12.4/8.12.4) with ESMTP id h4GDkH7A029508; Fri, 16 May 2003 15:46:17 +0200 Date: Fri, 16 May 2003 15:46:17 +0200 (CEST) X-X-Sender: derick@jdi.jdimedia.nl To: Holger Burbach cc: sander@php.net, PHP Internals In-Reply-To: <20030516133408.GA11228@lxr> Message-ID: References: <200305161525.04359.holger.burbach@gonicus.de> <20030516133408.GA11228@lxr> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Subject: Re: [PHP-DEV] PECL Package Proposal: kadm5 - REPOST From: derick@php.net (Derick Rethans) On Fri, 16 May 2003, Sander Roobol wrote: > On Fri, May 16, 2003 at 03:25:04PM +0200, Holger Burbach wrote: > > There is a new release of kadm5 with some minor changes. The sources can > > be found at: > > > > ftp://oss.gonicus.de/pub/php-kadm5/php-kadm5-0.2.2/ > > Looks fine, BUT (Derick, still reading? :) 1. The biggest problem I see is that the extension is throwing E_ERRORs which extensions really should not do if they do not leave *PHP* in an unstable state (there is also no \n required at the end of the text, and in the best case php_error_docref should be used). 2. Check our CODING STANDARDS file again :) 3. I would recommend to use this naming (As example): kadm5_principal_create kadm5_principal_modify instead of: kadm5_create_principal kadm5_modify_principal 4. There are no prototypes for the PHP_FUNCTIONs 5. There should be atleast one test per function, if possible. 6. The package file has some unescapted entities (the & in "Cleanup in kadm5.c: Removal of confirm_kadm5_compiled & RINIT/RSHUTDOWN" for example) regards, Derick -- "my other box is your windows PC" ------------------------------------------------------------------------- Derick Rethans http://derickrethans.nl/ PHP Magazine - PHP Magazine for Professionals http://php-mag.net/ -------------------------------------------------------------------------