Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Commit

Permalink
CS fixes per php-cs-fixer
Browse files Browse the repository at this point in the history
  • Loading branch information
weierophinney committed Feb 25, 2016
1 parent d98fa5b commit 4db6018
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 5 deletions.
3 changes: 1 addition & 2 deletions composer.json
Expand Up @@ -22,8 +22,7 @@
"zendframework/zend-console": "^2.6",
"zendframework/zend-di": "^2.6",
"zendframework/zend-loader": "^2.5",
"zendframework/zend-mvc": "^2.6.3",
"zendframework/zend-servicemanager": "^2.7.5 || ^3.0.3",
"zendframework/zend-servicemanager": "^3.0.3",
"fabpot/php-cs-fixer": "1.7.*",
"phpunit/PHPUnit": "~4.0"
},
Expand Down
1 change: 0 additions & 1 deletion test/Listener/DefaultListenerAggregateTest.php
Expand Up @@ -13,7 +13,6 @@
use Zend\ModuleManager\Listener\ListenerOptions;
use Zend\ModuleManager\Listener\DefaultListenerAggregate;
use Zend\ModuleManager\ModuleManager;
use ZendTest\ModuleManager\EventManagerIntrospectionTrait;

/**
* @covers Zend\ModuleManager\Listener\AbstractListener
Expand Down
1 change: 0 additions & 1 deletion test/Listener/LocatorRegistrationListenerTest.php
Expand Up @@ -16,7 +16,6 @@
use Zend\ModuleManager\ModuleManager;
use Zend\ModuleManager\ModuleEvent;
use Zend\Mvc\Application;
use Zend\ServiceManager\ServiceLocatorInterface;
use Zend\ServiceManager\ServiceManager;
use ZendTest\ModuleManager\TestAsset\MockApplication;

Expand Down
2 changes: 1 addition & 1 deletion test/Listener/OnBootstrapListenerTest.php
Expand Up @@ -60,7 +60,7 @@ public function setUp()
'ZendTest\Module\TestAsset\MockApplication',
'application',
]);

$this->application->setEventManager($appEvents);
}

Expand Down

9 comments on commit 4db6018

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really wanted in 2.7.0 ?
ZF2 requires zend-servicemanager ^2.5
So this component (requiring 3.0.3) can't be installed.

Especially the commit message "CS" seems to enlight some unwanted change.

@svycka
Copy link
Contributor

@svycka svycka commented on 4db6018 Feb 26, 2016

Choose a reason for hiding this comment

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

as this is in require-dev I don't think this is the case. No? And that should not prevent from being installed I think. But maybe I am wrong not 100% sure about this. And yes commit message and diff seems a bit strange :D

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

Right, this doesn't prevent installation, at least prevent to run the test suite.

@svycka
Copy link
Contributor

@svycka svycka commented on 4db6018 Feb 26, 2016

Choose a reason for hiding this comment

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

sorry don't get it what test suite, because https://travis-ci.org/zendframework/zend-modulemanager runs just fine with both 3.0 and 2.7 versions. Also I think all the required packages already supports 3.0 it wouldn't be any difference if it was ^2.7.5 || ^3.0.3 or just ^3.0.3

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

I encounter a:

  PHP Fatal error:  Class 'Zend\ModuleManager\Listener\ConfigListener' not found in /dev/shm/BUILD/zend-modulemanager-fe5a156df1ae4245c7b4a7b2b9067545761f59d7/test/Listener/ConfigListenerTest.php on line 44

I will dig more (probably related to path used here)

BTW, this .travis config which use --ignore-platform-reqs and force SERVICE_MANAGER_VERSION="^2.7.5" which is different from composer.json seems terribly ugly.

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

So related to some change in restoreAutoloadFunctions.
Reverting this function to the one from 2.6 solves it.

@weierophinney
Copy link
Member Author

Choose a reason for hiding this comment

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

Regarding the Travis config:

  • --ignore-platorm-reqs existed to allow testing against php 7 before composer supported the ||notation for dependency constraints. IIRC, it's also needed to allow installation on HHVM.
  • forcing the version is done to allow us to test against both v2 and v3 releases. I tried using the "prefer lowest" flag, but ran into issues with the test tooling whereby composer was unable to resolve all dependencies. The approach used here works reliably.

Regarding your statement:

Reverting this function to the one from 2.6 solves it

Can you explain what "this function" refers to? Where are you seeing an error, exactly?

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain what "this function" refers to? Where are you seeing an error, exactly?

I mean the "restoreAutoloadFunctions" fonction.
I'm mostly offline, and will try to provide more explanation / debug / fix next week.

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed by PR #32

Please sign in to comment.