Re: PECL Package Proposal: kadm5 - REPOST

On Donnerstag, 22. Mai 2003 15:04, Holger Burbach wrote: > Hi Derick, > > can you please have another look at my PECL module kadm5? I think that I > have fixed all the issues you mentioned. Sorry, the sources can be found at: ftp://oss.gonicus.de/pub/php/php-kadm5/php-kadm5-0.2.3/ > Well, hoping to get the final vote "+1" from you. ;-) > Thanks in advance. > > Bye > Holger > > On Friday, 16. Mai 2003 15:46, Derick Rethans wrote: > > 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). > > Fixed. > > > 2. Check our CODING STANDARDS file again :) > > Fixed. At least I hope so ;-) > > > 3. I would recommend to use this naming (As example): > > kadm5_principal_create > > kadm5_principal_modify > > > > instead of: > > kadm5_create_principal > > kadm5_modify_principal > > Well, I see your point, but changing the function names gets quite > confusing for someone who is familiar with the kadm5 C API. I would prefer > to keep them the way they are. > > > 4. There are no prototypes for the PHP_FUNCTIONs > > Fixed. > > > 5. There should be atleast one test per function, if possible. > > If someone has a clever idea, how to implement these test without accessing > a real Kerberos server, I would love to do it. > > > 6. The package file has some unescapted entities (the & in "Cleanup in > > kadm5.c: Removal of confirm_kadm5_compiled & RINIT/RSHUTDOWN" for > > example) > > Fixed.