Skip to content
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

Making cachetool compatible w/ php7 #49

Merged
merged 3 commits into from
Oct 1, 2017
Merged

Conversation

jrmbrgs
Copy link
Contributor

@jrmbrgs jrmbrgs commented Mar 7, 2017

  • Add support of php7.0-fpm socket
  • Fix some php7 warn
  • Skip unit tests related to APC (not supported by php7)

  * Add support of php7.0-fpm socket
  * Fix some php7 warn
  * Skip unit tests related to APC (not supported by php7)
@@ -21,6 +21,11 @@ class FastCGI extends AbstractAdapter
*/
protected $client;

protected $possibleSocketFiles = [
'/var/run/php5-fpm.sock',
'/var/run/php/php7.0-fpm.sock'
Copy link

Choose a reason for hiding this comment

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

/var/run/php-fpm.sock would be nice to add too, as a generic name.. or maybe glob for /var/run/php*.sock / /var/run/php/*.sock as a last resort?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, I pushed a bit too fast.
I'll change for both ['/var/run/php*.sock', '/var/run/php/php7.0-fpm.sock']

@@ -6,6 +6,9 @@ class ApcBinDumpCommandTest extends CommandTest
{
public function testCommand()
{
if (explode('.', PHP_VERSION_ID)[0] >= 7) {
Copy link

Choose a reason for hiding this comment

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

PHP_VERSION_ID >= 70000 reads a bit easier than this explode IMO :)

Copy link

Choose a reason for hiding this comment

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

Also you can't explode on '.' because PHP_VERSION_ID has no dots, it's an integer MAJOR/MINOR/PATCH without separator. See http://php.net/manual/en/reserved.constants.php#reserved.constants.core

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right, took the return of phpversion()...
Fixing that.

@jrmbrgs
Copy link
Contributor Author

jrmbrgs commented Mar 7, 2017

Really sorry, It missed one commit

@jrmbrgs
Copy link
Contributor Author

jrmbrgs commented Mar 7, 2017

@Seldaek Thanks for the reviews by the way.

@Grummfy
Copy link

Grummfy commented Apr 6, 2017

any reason why it's not merged?

@jrmbrgs
Copy link
Contributor Author

jrmbrgs commented Apr 25, 2017

I don't know, still waiting for it to be merged...

@robincee
Copy link

Your 6 commits behind master and it's not possible to merge your PR before you rebase / reroll the changes.

patch -p1 --dry-run < 49.patch
checking file src/CacheTool/Adapter/FastCGI.php
checking file src/CacheTool/Command/ApcCacheInfoCommand.php
checking file src/CacheTool/Command/ApcCacheInfoFileCommand.php
checking file tests/CacheTool/Command/ApcBinDumpCommandTest.php
checking file tests/CacheTool/Command/ApcBinLoadCommandTest.php
checking file tests/CacheTool/Command/ApcCacheClearCommandTest.php
checking file tests/CacheTool/Command/ApcCacheInfoCommandTest.php
checking file tests/CacheTool/Command/ApcCacheInfoFileCommandTest.php
checking file tests/CacheTool/Command/ApcKeyDeleteCommandTest.php
checking file tests/CacheTool/Command/ApcKeyExistsCommandTest.php
checking file tests/CacheTool/Command/ApcKeyFetchCommandTest.php
checking file tests/CacheTool/Command/ApcKeyStoreCommandTest.php
checking file tests/CacheTool/Command/ApcSmaInfoCommandTest.php
checking file src/CacheTool/Adapter/FastCGI.php
Hunk #1 FAILED at 21.
1 out of 1 hunk FAILED
checking file tests/CacheTool/Command/ApcBinDumpCommandTest.php
Hunk #1 FAILED at 6.
Hunk #2 FAILED at 21.
2 out of 2 hunks FAILED
checking file tests/CacheTool/Command/ApcBinLoadCommandTest.php
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED
checking file tests/CacheTool/Command/ApcCacheClearCommandTest.php
Hunk #1 FAILED at 6.
Hunk #2 FAILED at 19.
Hunk #3 FAILED at 31.
3 out of 3 hunks FAILED
checking file tests/CacheTool/Command/ApcCacheInfoCommandTest.php
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED
checking file tests/CacheTool/Command/ApcCacheInfoFileCommandTest.php
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED
checking file tests/CacheTool/Command/ApcKeyDeleteCommandTest.php
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED
checking file tests/CacheTool/Command/ApcKeyExistsCommandTest.php
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED
checking file tests/CacheTool/Command/ApcKeyFetchCommandTest.php
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED
checking file tests/CacheTool/Command/ApcKeyStoreCommandTest.php
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED
checking file tests/CacheTool/Command/ApcSmaInfoCommandTest.php
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED
checking file src/CacheTool/Adapter/FastCGI.php
Hunk #1 FAILED at 24.
Hunk #2 FAILED at 42.
2 out of 2 hunks FAILED

@jrmbrgs
Copy link
Contributor Author

jrmbrgs commented May 1, 2017

Hello @robincee,

I rebased and pushed.
Github now tells me that vpg:php7 is up to date with all commits from gordalina:master.

Thx you

@gordalina gordalina merged commit 42ae3c3 into gordalina:master Oct 1, 2017
@gordalina gordalina added this to the 3.0.0 milestone Oct 7, 2017
@jrmbrgs
Copy link
Contributor Author

jrmbrgs commented Oct 20, 2017

Thx !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants