Proposal for PEAR::PEAR_PackageUpdate

Scott Mattocks (http://pear.ph
p.net/user/scottmattocks) proposes
PEAR::PEAR_PackageUpdate.

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

-- 
Sent by PEPr, the automatic proposal system at http://pear.php.net

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

Scott Mattocks (http://pear.ph
p.net/user/scottmattocks) has edited the proposal for
PEAR::PEAR_PackageUpdate.

Change comment:

Removed PHP 5 public tokens from method signatures.

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

-- 
Sent by PEPr, the automatic proposal system at http://pear.php.net

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

Laurent Laville (http://pear.php.net/u
ser/farell) has commented on the proposal for
PEAR::PEAR_PackageUpdate.

Comment:

My first comment, is only related to source code and PHP
dependency.

A quick parse of source code with PHP_CompatInfo gave PHP
5.0.0 min but
this result is not significant.

So i've also tried with script :

<?php
require_once 'PEAR/Common.php';

$c = new PEAR_Common();
$file = '[...]PEAR\PackageUpdate.php';
$res = $c->analyzeSourceCode($file);
?>

And found unexpected public tokens on lines :

538:     public function setDontAskAgain($dontAsk)
555:     public function
setDontAskUntilNextRelease($nextrelease)
580:     public function setMinimumRleaseType($minType)
602:     public function setMinimumState($minState)
629:     public function setPreference($pref, $value) {

Need to fix it or this package is only for PEAR 2 and PHP
5.x 

Even Greg told us that PEAR 1.5.0 will have PHP 4.3.0 min.

Laurent Laville

Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=369

-- 
Sent by PEPr, the automatic proposal system at http://pear.php.net

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

Scott Mattocks a écrit :
> Scott Mattocks (http://pear.ph
p.net/user/scottmattocks) has edited the proposal for
PEAR::PEAR_PackageUpdate.
> 
> Change comment:
> 
> Removed PHP 5 public tokens from method signatures.
> 
> Please review the proposal:
> http://pear.php.net/pepr/pepr-proposal-show.php?id=369
> 

First cursory review (all code, not gTK2 because i'm not
expert), it 
looks nice ! Good Job 
I'll investigate deeper later

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

Laurent Laville (http://pear.php.net/u
ser/farell) has commented on the proposal for
PEAR::PEAR_PackageUpdate.

Comment:

Here are my latest comments, while looking deeper into
source code.

1. I think its dangerous to use on windows a preference file
named
"pear.ini" while you've named it
".ppurc" for other platforms.

remember "pear.ini" is default PEAR config file
name.

2. getPackageInfo() is suppose to return void (see phpdoc),
but in real
situation it could also return false on error (line 345).

3. why don't you used raiseError rather than new PEAR_Error
inside
pushError() method ?
It could made it easy with raiseError especially on lines
343-344 and
741-742
to transmit full pear_error instance

        if (PEAR::isError($result)) {
            $this->pushError($result);

4. Personnaly i prefer to see usage of constant that
identify an error and
a global mapping for error codes => error messages rather
messages all
other source code.

5. last you made a typo error in your example (into proposal
page and
source code line 57).

    if ($ppu->update() {

missing right close parenthesis.

Hope it will help and you understood me.

Laurent

Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=369

-- 
Sent by PEPr, the automatic proposal system at http://pear.php.net

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

Scott Mattocks (http://pear.ph
p.net/user/scottmattocks) has edited the proposal for
PEAR::PEAR_PackageUpdate.

Change comment:

Updates based on Laurent's comments.
- Add error constants.
- Allow error codes and PEAR_Error instances to be passed to
pushError.
- Update return values and docblocks.
- Fix Windows preference file name.
- Fix example in opening doc block.

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

-- 
Sent by PEPr, the automatic proposal system at http://pear.php.net

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

Thanks for the feed back Laurent.

Laurent Laville wrote:
> 
> 1. I think its dangerous to use on windows a preference
file named
> "pear.ini" while you've named it
".ppurc" for other platforms.
> 
> remember "pear.ini" is default PEAR config
file name.

Cut and paste mistake. Will fix ASAP. It should be ppurc.ini

> 
> 2. getPackageInfo() is suppose to return void (see
phpdoc), but in real
> situation it could also return false on error (line
345).

getPackageInfo() now returns a boolean in all cases.
(docblock fixed 
also). Actually, I have updated a bunch or other methods and
docblocks 
to have consistent return values also.

> 
> 3. why don't you used raiseError rather than new
PEAR_Error inside
> pushError() method ?
> It could made it easy with raiseError especially on
lines 343-344 and
> 741-742
> to transmit full pear_error instance
> 
>         if (PEAR::isError($result)) {
>             $this->pushError($result);

Good idea.

> 
> 4. Personnaly i prefer to see usage of constant that
identify an error and
> a global mapping for error codes => error messages
rather messages all
> other source code.

Another good point. That will help to standardize the error
messages also.

> 
> 5. last you made a typo error in your example (into
proposal page and
> source code line 57).
> 
>     if ($ppu->update() {
> 
> missing right close parenthesis.

Thank you. Fixed in the proposal and the source.

Thanks again,
-- 
Scott Mattocks
http://www.crisscott.com

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

Justin Patrin (http://pear.php
.net/user/justinpatrin) has commented on the proposal
for PEAR::PEAR_PackageUpdate.

Comment:

I would suggest using PEAR_ErrorStack instead of
implementing your own
error stack in the class.

I'm also not sure why you're not returning a PEAR_Error. I
don't see any
loops in the class itself so it doesn't make sense for it
to simply
aggregate its errors. The package should return
PEAR::raiseError on error
and let the calling package decide if it wants to aggregate
or not.

As has been said before, instanceOf is PHP5 only. If this is
a PHP4
package (as it seems to be) please use is_a.

Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=369

-- 
Sent by PEPr, the automatic proposal system at http://pear.php.net

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

Thanks for the comments Justin,

Justin Patrin wrote:
> 
> I would suggest using PEAR_ErrorStack instead of
implementing your own
> error stack in the class.

I am thoroughly confused by the docs for PEAR_ErrorStack but
I will try 
to read them over again and update the package.

> 
> I'm also not sure why you're not returning a
PEAR_Error. I don't see any
> loops in the class itself so it doesn't make sense for
it to simply
> aggregate its errors. The package should return
PEAR::raiseError on error
> and let the calling package decide if it wants to
aggregate or not.
> 

I am not returning errors because it makes the package more
difficult to 
use. By compiling a stack of errors, the user can write
something like 
the example in the proposal which I think is cleaner than:

$result = $ppu->checkUpdate();
if (!PEAR::isError($result)) {
     $result2 = $ppu->presentUpdate();
     ...
}

Returning PEAR_Error every time at least doubles the amount
of code 
needed to update a package.

Also, this package is designed as a back end for different
front end 
packages. I think most front ends will want to collect all
of the errors 
and then display them all at once like
PEAR_PackageUpdate_Gtk2 does. I 
am trying to put the common functionality into the base
package so that 
other packages don't have to implement it. If it will be
more difficult 
for other developers to work with the stack than to handle
errors on an 
individual basis I will change the package, but I don't see
the argument 
for that at the moment.

> As has been said before, instanceOf is PHP5 only. If
this is a PHP4
> package (as it seems to be) please use is_a.

instanceof is only in the PEAR_PackageUpdate_Gtk2 proposal
which is a 
PHP5 only package. PHP-GTK 2 requires at least PHP 5.1.2.

> 
> Proposal information:
> http://pear.php.net/pepr/pepr-proposal-show.php?id=369
> 

Thanks again for your comments.
-- 
Scott Mattocks
http://www.crisscott.com

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

Here are some answers for Scott, but also Christian Weiske
and Justin 
Patrin who asked two days ago why i've deleted my proposal
about a 
frontend for PEAR_PackageFileManager2.

I've talked with Scott about a common solution for my web
frontend and 
its own Gtk2 frontend : a backend as for PPU.

This class is almost finished, and before to propose it
officially to 
the PEAR community with a revisited copy of my old web
frontend, i'll
test it with Scott.

BTW i open NOW a call for comment about a CLI frontend for
future 
PEAR_PackageFileManager2. Do you think its possible ? does
anyone have 
already ideas ? All feedback are welcome !!!

Scott, i'll send to you a preview of PFM backend with a
PEAR_ErrorStack 
implementation, i've used. Something very similar to your
PPU need.

I've also used a contribution (not official) part of code
of StackThunk 
package from Ian Eure. I hope it won't be a problem if i
use part of 
code: especially PEAR_Error repackaging. And of course give
a credit to Ian.

Laurent Laville

PS: Greg Beaver, if you read this post, i'm still waiting a
answer about 
  my help request from PFM package 2.0 and install
conditions.


Scott Mattocks a écrit :
> Thanks for the comments Justin,
> 
> Justin Patrin wrote:
>>
>> I would suggest using PEAR_ErrorStack instead of
implementing your own
>> error stack in the class.
> 
> I am thoroughly confused by the docs for
PEAR_ErrorStack but I will try 
> to read them over again and update the package.
> 

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