search

tedious / Stash

The place to keep your cache.
http://www.stashphp.com
BSD 3-Clause "New" or "Revised" License
961 stars 133 forks source link

Apc Driver: Check for 'apc.enable_cli' in cli calls #368

Closed Biont closed 7 years ago

Biont commented 7 years ago

As described in #365, Stash can currently run into a fatal error in specific circumstances.

This PR introduces a check in Apc->isAvailable() to let the driver fail early if APCu is not configured to run in CLI environments.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.07%) to 95.468% when pulling cd196fd6da673af628d0c84cef18a099827eb93e on Biont:master into e9a6aaf7039ded487e91ba521d5c1573ff50acaa on tedious:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.07%) to 95.468% when pulling cd196fd6da673af628d0c84cef18a099827eb93e on Biont:master into e9a6aaf7039ded487e91ba521d5c1573ff50acaa on tedious:master.

alexbowers commented 7 years ago

Can you confirm that this works as expected, both with and without APC enabled?

Biont commented 7 years ago

I did test it locally both with and without apc.enable_cli in my php config, yes.

Biont commented 7 years ago

Hm, is there anything I need to do to make Travis happy? I checked for tests of the isAvailable method and found none.

Since ini_get() is not testable and introducing a config container for abstraction seemed out of scope, I did not touch unit tests.

tedivm commented 7 years ago

@Biont - the test that's failing is the style check. You just need to fix the indentation on your code and it should pass.

@alexbowers - feel free to merge this once it passes.

alexbowers commented 7 years ago

@tedivm Is it the style?

Could be worth splitting up between StyleCI and Travis CI for style and unit tests?

tedivm commented 7 years ago

Yeah, when I set the project up StyleCI didn't exist so I just added a test case in for styling. If you want to break that up that would be great.

alexbowers commented 7 years ago

do you have a particular style guide you follow? (Eg. PSR etc)

On 23 August 2017 at 22:24, Robert Hafner notifications@github.com wrote:

Yeah, when I set the project up StyleCI didn't exist so I just added a test case in for styling. If you want to break that up that would be great.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tedious/Stash/pull/368#issuecomment-324466911, or mute the thread https://github.com/notifications/unsubscribe-auth/AAzc3sx0iYDiuk-kf284lklpr6rlF6g1ks5sbJiWgaJpZM4O9aAD .

tedivm commented 7 years ago

Yeah- it's all in the contributing docs.

tedivm commented 7 years ago

And here - https://github.com/tedious/Stash/blob/master/.php_cs

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.07%) to 95.468% when pulling 0ae4b37061f4d5caba7d38a96866130a9b9f33fd on Biont:master into e9a6aaf7039ded487e91ba521d5c1573ff50acaa on tedious:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.07%) to 95.468% when pulling 0ae4b37061f4d5caba7d38a96866130a9b9f33fd on Biont:master into e9a6aaf7039ded487e91ba521d5c1573ff50acaa on tedious:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.07%) to 95.468% when pulling 0ae4b37061f4d5caba7d38a96866130a9b9f33fd on Biont:master into e9a6aaf7039ded487e91ba521d5c1573ff50acaa on tedious:master.

Biont commented 7 years ago

Thanks for the hint @tedivm I enabled PSR2 codesniffing for this project and should be good to go now, should there be future PRs

alexbowers commented 7 years ago

Thanks @Biont.

@tedivm merged.