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

Add missing error control operator prefix '@' before ini_set() #8830

Merged
merged 1 commit into from Sep 21, 2015

Conversation

Projects
None yet
4 participants
@Joey3000
Contributor

Joey3000 commented Sep 21, 2015

Background: #8824 (comment)

This PR adds @ where missing before ini_set().

Important:
The ini_set() presence check-and-show-error block in https://github.com/piwik/piwik/blob/2.15.0-b8/core/testMinimumPhpVersion.php#L35 is not being modified by this PR. Please let me know if it needs to be removed.

@tsteur

This comment has been minimized.

Show comment
Hide comment
@tsteur

tsteur Sep 21, 2015

Member

Merging this one, if you're keen on refactoring the ini_set method out into something reusable to avoid having this problem again in the future feel free to go for it :) Could be a simple Piwik\PhpIni class having a set method but not needed to do :)

Member

tsteur commented Sep 21, 2015

Merging this one, if you're keen on refactoring the ini_set method out into something reusable to avoid having this problem again in the future feel free to go for it :) Could be a simple Piwik\PhpIni class having a set method but not needed to do :)

tsteur added a commit that referenced this pull request Sep 21, 2015

Merge pull request #8830 from Joey3000/addErrorControl
Add missing error control operator prefix '@' before ini_set()

@tsteur tsteur merged commit 4f1da08 into matomo-org:master Sep 21, 2015

2 checks passed

Scrutinizer No new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Joey3000 Joey3000 deleted the Joey3000:addErrorControl branch Sep 21, 2015

@mattab mattab added the Bug label Oct 13, 2015

@cdwiegand

This comment has been minimized.

Show comment
Hide comment
@cdwiegand

cdwiegand Nov 11, 2015

Can we remove the ini_set check entirely? We also have ini_set disabled for security reasons (WordPress blog hacks LOVE to use ini_set to hide themselves from causing errors), and so I have to patch piwik every release internally before I can push out updates. I find piwik works just fine with ini_set disabled so long as session handling is working correctly.

cdwiegand commented Nov 11, 2015

Can we remove the ini_set check entirely? We also have ini_set disabled for security reasons (WordPress blog hacks LOVE to use ini_set to hide themselves from causing errors), and so I have to patch piwik every release internally before I can push out updates. I find piwik works just fine with ini_set disabled so long as session handling is working correctly.

@mattab mattab added the answered label Oct 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment