Re: [PEPr] -1 for Math::BigInteger

From: Date: Tue, 27 Jun 2006 19:00:53 +0000
Subject: Re: [PEPr] -1 for Math::BigInteger
References: 1 2 3  Groups: php.pear.dev 
Request: Send a blank email to pear-dev+get-43157@lists.php.net to get a copy of this message
I find it somehow unconstructive (if not destructive) to come up with such things in the vote phase. The proposal has been online for 2 weeks. CS Issues can be fixed in CVS.
I'm sorry that nobody looked at this sooner. Anyone from PEAR who took two minutes to look over the code would notice these glaring issues (they're not difficult to fix, but glaring nonetheless). Don't poo poo me for catching them a little late. As they say, better late than never.
The bcpowmod function you define should either be a part of PHPCompat (ie. your package requires PHPCompat, which implements this function) or you should put that if statement into your class definition.
Jim does not have to use PHPCompat, you can if he feels that it will help (I personnally would not use it here).
My point probably wasn't clear. In his example script he checks for a function and then defines it if it's not there. If his package relies on that function then it should be in the package files and not the example / userland code.
Ideally, your class would use the native BC functions if they existed as well.
I would rather go directly with an early version (4.3.x for example).
What do you mean by this? I was saying that his class should use native PHP BC functions if they exist for performance reasons. I didn't make any mention of PHP versions so I'm a bit confused as to what you're saying here.
It would be nice if further votes (in pepr in general) focus on the goal and implementation of a package before voting -1, especially when all bad points are about CS, inline docs and PHPCompat.
It would have been preferable to have people comment during the earlier phase, but obviously that didn't happen. It's better it's caught later in the voting phase than not caught at all. --Joe

« previous php.pear.dev (#43157) next »