Hi,
2015-03-10 21:31 GMT-03:00 Stanislav Malyshev <smalys...@gmail.com>:
> Hi!
>
> > - Even if you already read the RFC in the past, read it again now.
> > - Don't claim **possible** massive BC breaks before read the
> > measurements already done. No matter how seasoned you are with PHP,
> real
> > numbers matter most than assumptions. Your measurements are welcome
> too.
>
> I'd say your own data - 4597 warnings on 27 different cases of calls -
> is quite fitting the "massive BC break" category. If I had some app and
> had to evaluate the upgrade to PHP 7 and would see that it produces
> thousands of errors and I have to patch dozens of separate places to fix
> it (your data doesn't say if the sources of the calls are the same, so I
> take the best interpretation - that both source and target are the same)
> - I would postpone the upgrade as far as possible, because it looks like
> a huge disruption.
>
Have you read the rest of the paragraph you distorted?
Because you just took a slice of the paragraph, presented it out of context
like if everybody will just buy your malformed misinterpretation without go
check the RFC and see:
[...]
Running the test suite resulted in a slay of *4.597* warnings directly
related to the proposed RFC. *But* after some heuristics it was noticeable
that most warnings had a common cause. I parsed the output file to remove
duplicated warnings and the result was a derisive number of *only 27 unique
warnings*. You can see the condensed list of unique warnings below:
[...]
After checking case by case among the *27 unique* warnings found, the
conclusion is that *all of them* are a result of *human mistake* from some
code refactory that left function calls with residual parameters behind.
This could be fixed in less than 1 hour.
[...]
The wordpress test suite is basically a bunch of integration tests. If you
have a single warning
it will repeat 4 thousand times, if you fix this warning it disappears from
all 4 thousand tests.
I even parsed the output file to prove that it's only 27 warnings, not the
apparent 4.597 warnings
you decided to quote out of context. It was clearly proven that the BC
break is very isolated even
in a massive codebase like wordpress.
Fixing 27 function calls won't impede Wordpress or anyone to migrate to
PHP7.
It's an easy fix that could prevent nasty dangerous bugs later.
> Additionally, ZF2 tests couldn't even run - and from this you conclude
> that your patch is fine. I conclude that we already have a big problem,
> and making it worse is not a good idea. If we do it couple of more
> times, we'd probably get people to run PHP 7 somewhere in 2025.
>
>
The ZF2 test suite couldn't run because of fatal errors caused by PHP7
incompatibilities
or bugs that caused segfaults. Nothing related to the RFC itself.
Hope you don't expect to prove a point just by saying that an isolated BC
break will make
the PHP7 migration "much worse" based solely on your instincts about a test
suite that didn't
even run - for unrelated reasons to the RFC proposal - (look at the other
test suites that ran fine).
I measured the BC breaks responsibly before claim anything. Try to do the
same.
That's not enough effort of your part on a mailing list discussion, sorry.
> Additionally, I feel bad about magic features that change depending on
> which code is inside the function. That doesn't feel like a good design.
>
If compilation is "magic"... we are all magicians here. Calling the
implementation "magic"
won't change the fact that it's just a compilation check :D
> Additionally, there is more than variadic functions which may use extra
> parameters. Such as dynamic dispatch:
>
> function getVariables($vars, $arg) {
> foreach($vars as $var) {
> $getter = "get$var";
> $result[] = $this->$getter($arg);
> }
> return $result;
> }
>
> grossly simplified of course. Now imagine some getters need the arg and
> some don't. I can't write this code anymore without either:
> 1. Adding argument to all getters even though they don't need it
> (unclean and confusing)
> 2. Checking how many arguments each getter needs and modifying the loop
> accordingly (even more confusing)
>
>
If you plan to use methods|functions interchangeably then you should make
them compatible
from now on. That's one of the points of the RFC. It's not "unclean".
BTW, the current PHP silent behavior should be considered even more
confusing otherwise we
wouldn't have these measurements:
https://wiki.php.net/rfc/strict_argcount#bc_breaks_on_the_real_world
It basically means that even experienced package maintainers are shooting
on their own foot and
they are using code evaluation tools, static analyzers, etc. Now imagine
the common PHP programmer.
> > - Try the patch. Really.
> > - Consider reading the use case present on this post: goo.gl/3ykdIy
>
> The case you bring there looks artificially constructed to support this
> RFC. If your unit tests are so bad as to not actually check that the
> functions like createUser() do actually create the user you asked to
> create and don't just drop information on the floor, and your code
> refactoring practices are so bad as to allow people just drop arguments
> from public APIs and replace them with other arguments, and all this
> without checks or typing - parameter counts is not what is going to save
> you. Your own example would break if both changes are done at the same
> time (if you can do either of them, no reason why you can't do both).
>
>
There is nothing bad about using examples to illustrate something, just
because
the example is "artificial" it doesn't mean it's disconnected from reality.
The example
on goo.gl/3ykdIy is far from being a lie, it's indeed an accurate example.
You can't claim that I artificially built the test suites of a lot of open
source
projects just to "support the RFC". The test suites (and the code being
tested)
grew quite "organically", I just run them to do some measurements:
- and there it is: the same patterns from the "artificial" example were
detected
numerous times "on the wild".
> --
> Stas Malyshev
> smalys...@gmail.com
>
Márcio
