[PEPr] Proposal for XML::Create_KML

[Robert McLeod]

Robert McLeod (http://pear.php.net/user/hamstar) proposes XML::Create_KML.

You can find more detailed information here:
 http://pear.php.net/pepr/pepr-proposal-show.php?id=616

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[Robert McLeod]

Robert McLeod (http://pear.php.net/user/hamstar) has initiated the call for votes on XML::Create_KML.

Please review the proposal and give your vote here:
http://pear.php.net/pepr/pepr-proposal-show.php?id=616

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[tfk]

On Sat, Oct 10, 2009 at 11:34 AM, Robert McLeod <hamstar@...> wrote:
>
> Robert McLeod (http://pear.php.net/user/hamstar) has initiated the call for votes on XML::Create_KML.
>
> Please review the proposal and give your vote here:
> http://pear.php.net/pepr/pepr-proposal-show.php?id=616
>

Hey Robert,

the proposal process is vital to releasing in PEAR. Don't you want to
ask for comments before people vote? There's a few things where I'd
have to vote 'nay' right away.

Till

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[David Coallier-4]

2009/10/11 till <klimpong@...>:

> On Sat, Oct 10, 2009 at 11:34 AM, Robert McLeod <hamstar@...> wrote:
>>
>> Robert McLeod (http://pear.php.net/user/hamstar) has initiated the call for votes on XML::Create_KML.
>>
>> Please review the proposal and give your vote here:
>> http://pear.php.net/pepr/pepr-proposal-show.php?id=616
>>
>
> Hey Robert,
>
> the proposal process is vital to releasing in PEAR. Don't you want to
> ask for comments before people vote? There's a few things where I'd
> have to vote 'nay' right away.
>
> Till
>
> --
> PEAR Development Mailing List (http://pear.php.net/)
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Yeah in fact I'm setting up a git repository called pear-reviews and
we have your code restructured to look along the lines of what the
format should like.

Asking for comments before voting is definitely a good step. I can see
a few vital things that will get you immediate -1 thus refusing the
package.


--
Slan,
David

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[Robert McLeod]

OK then how do I set it back to the comments stage, the edit button
has disappeared.

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[Till Klampaeckel-2]

First off, there are a lot of reasons why you should never race to a 1.0.
The most important is that alpha and beta versions allow you to adjust and
improve the API, while this is nearly impossible once stable.



Then on to a couple comments on the code:



1) First off, one-class-per-file rule. This means that all your classes
should probably be called KML_*. Or if it's acceptable, XML_KML_*, e.g.
XML_KML_Place, vs. KMLPlace, this results in a path name,
XML/KML/Place.php.



2) It would be much nicer if you use used xmlwriter or simplexml to create
your xml.



3) Unit tests would be very much appreciated, and I think they shouldn't
be too hard to come up with since the result seems relatively straight
forward.



4) You don't want to send headers in PHP code (because of among all the
things why this is not a good idea, making it harder to unit test your code
is one of the most obvious).



5) KMLStyle (or well, XML_KML_Style, or KML_Style), is not a bad idea, but
I'd appreciate set*() and get*() that also validate input when needed, when
needed. I'd suggest you implement those in KMLPlace, etc. as well.



6) For addItem(), you could force e.g. XML_KML_Common in the signature:

function addItem(XML_KML_Common $item), then all the different objects
extend from XML_KML_Common and you created a simple object hirachey. Could
also make sense to add addStyle() (or maybe setStyle()) for the different
types of objects.



7) In some cases exceptions wouldn't hurt -- e.g. for invalid arguments.
FALSE is too plain, and since the methods can return FALSE for various
reasons, it becomes tedious to debug.



8) With a lot of add*()/set*(), a fluent interface would be nice.



9) create() -- personally, I'd implement __toString() which would replace
create() completely (or act as a wrapper).



10) Not sure if save() is needed.



11) I can see cloning come in handy -- so I suggest you implement the
magic method.



12) Maybe a destructor to clean up?



13) Please implement empty constructors.



14) Last but not least, run PHP_CodeSniffer to fix all the obvious issues
and to confirm to our coding standard.





Anyway, as David said, he's setting up a sandbox on Github. You could
commit your code and work on it and a couple of us could help you improve
this so you can propose this code to PEAR.



Let me know if you have any Qs!



Till

--
http://pear.php.net/pepr/pepr-proposal-show.php?id=616

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[Robert McLeod]

OK cool I'm adjusting some things now... Just a few questions:



3) What is a unit test?



4) Its not whether I want to send headers it whether the user of the
script wants to send headers.  Sometimes it is handy to show XML in the
styled view in firefox.  Right?



6) How can KML_Place and KML_Style extend from KML_Common when they are
comletely different objects?



9) __toString is awesome!



I'll have to look into cloning and exceptions before I implement them
because I don't really understand them...



Cheers for the feedback :)

--
http://pear.php.net/pepr/pepr-proposal-show.php?id=616

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[tfk]

On Fri, Oct 16, 2009 at 8:24 PM, Robert McLeod <hamstar@...> wrote:
>
> OK cool I'm adjusting some things now... Just a few questions:
>
> 3) What is a unit test?

E.g., through phpUnit.

(I know this will be a steep learning curve, but it's well worth it.)

1) Install phpunit (from http://phpunit.de/)
2) An example testcase

// tests/KMLTestCase.php
class KMLTestCase extends PHPUnit_Framework_TestCase
{
   protected $kml;
   public function setUp()
   {
      // Create a new KML object
      $this->kml = new KML;
   }
   public function testIfItWorked()
   {
      $this->kml->doSomething();
      $this->assertEquals('<xml .../>', $this->kml->whatever());
   }
}

3) Run with phpunit:
phpunit KMLTestCase.php

This is more or less pseudo code, which serves as an example, and is
not meant to work. :-)

> 4) Its not whether I want to send headers it whether the user of the
> script wants to send headers.  Sometimes it is handy to show XML in the
> styled view in firefox.  Right?

Yeah, I agree. I was just saying that maybe not do it in the class. ;-)

> 6) How can KML_Place and KML_Style extend from KML_Common when they are
> comletely different objects?

Like that:

//KML/Common.php:
class KML_Common
{}

//KML/Place.php
require_once 'KML/Common.php';
class KML_Place extends KML_Common
{}

>
> 9) __toString is awesome!
>
> I'll have to look into cloning and exceptions before I implement them
> because I don't really understand them...
>
> Cheers for the feedback :)
>

Let me know where you need help!

Till

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[Robert McLeod]

Robert McLeod (http://pear.php.net/user/hamstar) has edited the proposal for XML::Create_KML.

Change comment:

Various fixes and changes as suggested from the comments on proposal.

* The function to create the KML code now uses the SimpleXML class.

* Function to create KML code is now in the magic __toString() method
under the XML_KML_Create

* Removed the save() method

* Implemented constructers and destructers

* Implemented one-class-per-file rule adding Main.php KML/Place.php and
KML/Style.php

* Changed class name KML to XML_KML_Create

* Changed class name KMLPlace to XML_KML_Place

* Changed class name KMLStyle to XML_KML_Style

* Implemented set*() methods on XML_KML_Style and XML_KML_Place that
validate input

* Code now adheres to PHPCodeSniffer standards

Please review the proposal:
http://pear.php.net/pepr/pepr-proposal-show.php?id=616

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[Till Klampaeckel-2]

Your package is broken. I can't untar etc. it when I download it.



Also, would you mind putting all your code on Github? It makes easier
looking through it.



Till

--
http://pear.php.net/pepr/pepr-proposal-show.php?id=616

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[Robert McLeod]

Robert McLeod (http://pear.php.net/user/hamstar) has edited the proposal for XML::Create_KML.

Change comment:

Updated the Package so it works now.

Please review the proposal:
http://pear.php.net/pepr/pepr-proposal-show.php?id=616

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[Till Klampaeckel-2]

I'm including pear-dev.

I'll fork and make some changes, to push you in the right direction.
:) You'll just have to merge back using Github's forkqueue (super
easy).

Till

On Sun, Oct 18, 2009 at 1:06 PM, Robert McLeod <hamstar@...> wrote:

> Hey I fixed the package and also uploaded the project to github.  Was
> much easier than PEAR...
>
> Here's the link: http://github.com/hamstar/Create_KML
>
> Cheers :)
>
> On 10/17/09, Till Klampaeckel <till@...> wrote:
>>
>> Your package is broken. I can't untar etc. it when I download it.
>>
>> Also, would you mind putting all your code on Github? It makes easier
>> looking through it.
>>
>> Till
>>
>> --
>> http://pear.php.net/pepr/pepr-proposal-show.php?id=616
>>
>

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

[Till Klampaeckel-2]

I made some changes, and demo'd how a fluent interface works (e.g. see
XML_KML_Create::addItem(), or all set*() in XML_KML_Place). Also fixed
up cs a little, added XML_KML_Common (see _Place/_Style), added an
exception class, and so on.

If you want to talk, I'm on IRC (efnet): #pear, or we could also chat
on jabber (klimpong'atsymbol'gmail'dot'com).

Till

On Sun, Oct 18, 2009 at 5:32 PM, till <till@...> wrote:

> I'm including pear-dev.
>
> I'll fork and make some changes, to push you in the right direction.
> :) You'll just have to merge back using Github's forkqueue (super
> easy).
>
> Till
>
> On Sun, Oct 18, 2009 at 1:06 PM, Robert McLeod <hamstar@...> wrote:
>> Hey I fixed the package and also uploaded the project to github.  Was
>> much easier than PEAR...
>>
>> Here's the link: http://github.com/hamstar/Create_KML
>>
>> Cheers :)
>>
>> On 10/17/09, Till Klampaeckel <till@...> wrote:
>>>
>>> Your package is broken. I can't untar etc. it when I download it.
>>>
>>> Also, would you mind putting all your code on Github? It makes easier
>>> looking through it.
>>>
>>> Till
>>>
>>> --
>>> http://pear.php.net/pepr/pepr-proposal-show.php?id=616
>>>
>>
>

--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php