-
Notifications
You must be signed in to change notification settings - Fork 8k
Change pm.start_servers default warning to notice #895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Unlike other settings, this has a perfectly reasonable default, calculated using a dynamic formula. If the default was hardcoded to "2" or something, then it would make sense to have a warning, since that could potentially be bad, but for a dynamically calculated value based on other mandatory settings, a notice ought to be enough.
|
Seems ok to me. Can you please open a bug to link this change with. |
|
Having a unit test will also be appreciated |
|
Okay so this fix doesn't actually work. The reason is that the warning is emitted in I propose to move the call to Does that sound reasonable, @remicollet ? |
|
Alternatively, leave it in both places. Either way this kind of renders the other (already merged) PR obsolete, but no harm done I guess. |
|
Or should I re-open that other PR with an amended fix, and we leave this one here separate? Harder to test and merge though. Please advise. |
|
Best way is to provide all needed changes in this PR (the other is merged/closed) |
|
Yup, just saying that this change moves the location of where |
|
PR updated with newly moved |
|
Comment on behalf of remi at php.net: merged |
|
Thanks for your contribution. |
|
Thanks @remicollet. I was wondering the same about the tests (have one in warning level and one in notice) and meant to ask you if I should do that but forgot :) |
|
Could you also merge this to PHP-5.5 please, like the other change? Thanks @remicollet! |
|
Merging in 5.5 is not possible for test (rely on -O which is 5.6+) |
* PHP-5.6: Raise a warning when listen = hostname used and is resolved as multiple addresses NEWS Fixed #68458 Change pm.start_servers default warning to notice fix test description tests for #895 move zlog_set_level() again Change pm.start_servers default warning to notice
Unlike other settings, this has a perfectly reasonable default, calculated using
a dynamic formula. If the default was hardcoded to "2" or something, then it
would make sense to have a warning, since that could potentially be bad, but for
a dynamically calculated value based on other mandatory settings, a notice ought
to be enough.