Re: [PEPr] +1 for Images::Image_QRCode

On Fri, Dec 25, 2009 at 8:04 PM, Till Klampaeckel <till@php.net> wrote: > > Till Klampaeckel (http://pear.php.net/user/till) has voted +1 on the > proposal for Images::Image_QRCode. > > Proposal information: > http://pear.php.net/pepr/pepr-proposal-show.php?id=623 > Vote information: > —Ì®! > ZvúŒH¢6F4http://pear.php.net/pepr/pepr-vote-show.php?id=623&handle=till > > This vote is conditional. The condition is: > > Hey Rich, > > I briefly looked and here are my suggestions which make this vote > conditional: > > - Image_QRCode::makeCode() needs heavy refactoring > - @access tags are redundant > - all your set*() methods in (Image_QRCode) could be public and "return > $this;" (to provide a fluent interface) > - "data" and "image" directories should be on the top level like > "doc" (but > anyway, kudos for a complete package.xml) > - your API version is to high > - unit tests that cover the current feature set are needed > > I look forward to this code in PEAR! > > Happy holidays, > Till > Hi Till and others! Thanks for the comments on this proposal - I'm working on the tests at the moment, followed by the makeCode() refactoring. Re the set*() methods - what would be preferable in terms of an API? I've moved the configuration methods to an array of options as suggested earlier on on pear-dev, but as a result I moved the set*() methods to protected rather than public. I'm happy to move these back to public, but I wanted to check the general consensus/opinion on the recommended API for this package? Thanks and best wishes for the season! Rich. -- Rich Sage Oxford, UK rich.sage@gmail.com