Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Fix ini_get() for boolean values#29020

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:2.8fromdeguif:2.8
Oct 31, 2018
Merged

Fix ini_get() for boolean values#29020

fabpot merged 1 commit intosymfony:2.8fromdeguif:2.8
Oct 31, 2018

Conversation

@deguif
Copy link
Contributor

@deguifdeguif commentedOct 29, 2018•
edited
Loading

QA
Branch?2.8
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets
LicenseMIT
Doc PR

Currently settingfalse oroff, ... value to configure some PHP ini directives will make this evaluated totrue as this is equal to a non empty string.

Copy link
Contributor

@ndenchndench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM 👍

protectedfunctionsetUp()
{
if (!(ini_get('apc.enabled')&&ini_get('apc.enable_cli'))) {
if (!(\filter_var(ini_get('apc.enabled'), \FILTER_VALIDATE_BOOLEAN)&&\filter_var(ini_get('apc.enable_cli'), \FILTER_VALIDATE_BOOLEAN))) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Can you please remove all the\ added in front offilter_var() andFILTER_VALIDATE_BOOLEAN in the PR?
We don't add these except for a specific short list of functions.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ok, changed ;)

@ro0NL
Copy link
Contributor

See also

array('OPcache',\extension_loaded('Zend OPcache') &&ini_get('opcache.enable') ?'true' :'false'),
array('APCu',\extension_loaded('apcu') &&ini_get('apc.enabled') ?'true' :'false'),

}elseif ($displayErrors && (!ini_get('log_errors') ||ini_get('error_log'))) {
also maybe

@ro0NL
Copy link
Contributor

oh wait.. i checked master :) AboutCommand would be for 3.4

@deguif
Copy link
ContributorAuthor

Yes, will do another PR which will target 3.4 after ;)

@deguif
Copy link
ContributorAuthor

I'm also going to check every ini_get, I just focused onopcache andapc for the moment, going to send them ASAP

@ro0NL
Copy link
Contributor

ro0NL commentedOct 30, 2018•
edited
Loading

Cool :) i was curious if this applies to all boolean typed ini directives. I think it does 👍https://3v4l.org/aYsiU

@deguif
Copy link
ContributorAuthor

Ok, just added

  • log_errors
  • xcache.cacher
  • wincache.ocenabled

Not sure abouteaccelerator.enable as it doesn't use the macroSTD_PHP_INI_BOOLEAN

@fabpot
Copy link
Member

Thank you@deguif.

@fabpotfabpot merged commita153869 intosymfony:2.8Oct 31, 2018
fabpot added a commit that referenced this pull requestOct 31, 2018
This PR was merged into the 2.8 branch.Discussion----------Fix ini_get() for boolean values| Q             | A| ------------- | ---| Branch?       | 2.8| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets || License       | MIT| Doc PR        |Currently setting `false` or `off`, ... value to configure some PHP ini directives will make this evaluated to `true` as this is equal to a non empty string.Commits-------a153869 Fix ini_get() for boolean values
@deguifdeguif deleted the 2.8 branchOctober 31, 2018 08:57
@deguifdeguif restored the 2.8 branchOctober 31, 2018 09:16
This was referencedNov 3, 2018
nicolas-grekas added a commit that referenced this pull requestNov 6, 2018
This PR was merged into the 3.4 branch.Discussion----------Fix ini_get() for boolean values| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets || License       | MIT| Doc PR        |This follows#29020 for branch 3.4Commits-------65b34cb Fix ini_get() for boolean values
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+2 more reviewers

@ndenchndenchndench approved these changes

@ogizanagiogizanagiogizanagi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

2.8

Development

Successfully merging this pull request may close these issues.

7 participants

@deguif@ro0NL@fabpot@nicolas-grekas@ndench@ogizanagi@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp