Hi!
I looked at pull 638 and it looks OK code-wise. Are there any objections
still to merging it? It changes behavior slightly and the warning in the
docs no longer applies, but I think new behavior is actually better than
the old one as for most doubles between -1 and 1 we do not get random
sorting behavior anymore.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
On Sun, Apr 6, 2014 at 8:28 PM, Stas Malyshev smalyshev@sugarcrm.comwrote:
Hi!
I looked at pull 638 and it looks OK code-wise. Are there any objections
still to merging it? It changes behavior slightly and the warning in the
docs no longer applies, but I think new behavior is actually better than
the old one as for most doubles between -1 and 1 we do not get random
sorting behavior anymore.
I'm not entirely sure if this is such a good idea, but I also can't say
it's a bad idea.
It'll likely break someone's code out there that relies on the existing
behavior (for some strange reason). I thought it was actually a better idea
to document the behavior. Which is why I added the caution in the manual.
It's simpler and removes the ambiguity about what's expected from the
callback.
This PR just adds some complexity for little more than added magic. I'm not
a fan of magic in general, but I can see how most users think of magic as
being useful... That is of course, until it comes back to bite them.
Not opposed to merging this, but just my 2 cents.