On Tue, Oct 18, 2016 at 11:08 PM, Stanislav Malyshev <smalys...@gmail.com> wrote:
> Hi! > > > I pushed patch fixes number of nonsense/inconsistent session function > > behaviors. The additional patch is pushed so that it's easy to cherry > > pick minimum fixes. The last push is the additional fixes. > > These changes look like a reasonable cleanup. I'm not a big fan of > zend_parse_parameters_none additions, since I don't see any useful > function for them, but they don't hurt either. I left some notes on the > patch though, please look there. > > I'm not sure about 7.1 - while I don't see anything that would break any > reasonable code, except issues noted on the pull, the patch is pretty > big and I might have missed something. I guess it's for RM to decide. No > objection for 7.2. > > I notice that most test changes deal with scenarios that were covered > before, are there tests that test the new things - the scenarios for > which handling has changed? I couldn't find tests for some cases, like > INI modification, would be nice to add them. > > Also, a reminder that you'd have to update all the documentation in the > manual to reflect the change in the return values. And UPGRADING too. > I was planning to fix session_start() behaviors by PHP 7.1, but I > forgot to do this completely. Partial fix is merged currently. Yasuo, assuming "partial fix" doesn't mean "broken fix" but instead "it doesn't do everything I planned" then I do not want this in 7.1. As others have pointed out, it's not a small change and sessions are a fundamental part of PHP — we are simply too far into the release cycle to include this. Please target 7.2/master. Thanks, - Davey
