Comments for "Services_Libravatar"

» Submit Your Comment
Comments are only accepted during the "Proposal" phase. This proposal is currently in the "Finished" phase.
» Comments

  • Christian Weiske  [2011-05-19 08:11 UTC]

    I think libravatar is great and welcome a pear package for it.

    Your code nevetheless needs some ground work:
    - Since you are interfacing a web service, use Services_Libravatar as package name
    - Give the class the proper class names (Services_Libravatar), put them in the correct place
    - You don't need "@access" docblock tags anymore since we're on php5
    - You could cache DNS results, which make using an libravatar object for several email addresses faster
    - Please have a look at Net_DNS and if you can use it. Maybe it already includes sorting by priority, so you don't have to use your own implementation.

  • Michael Gauthier  [2011-05-19 18:53 UTC]

    Hi. This is a great start.

    1.) run phpcs on your code to find many small issues with coding standards

    2.) your example code creates a new object and calls a method on the object but the URL method is documented as @static and uses $this::foo() internally. I recommend just using non-static method access.

    3.) The organization of the code doesn't match the PEAR standards. You should have:

    /
    -docs/
    -LICENSE
    -package.xml
    -Services/
    --Libravatar.php