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

PHP 7.2 and Symfony ^4.0 #52

Merged
merged 11 commits into from
Dec 13, 2017

Conversation

UFOMelkor
Copy link
Member

@UFOMelkor UFOMelkor commented Dec 7, 2017

On travis:

  • PHP 7.2 runs against highest dependencies
  • PHP 7.1 runs against lowest dependencies

Some things need to be adjusted to get it working for symfony 4.

  • DataCollectorInterface has a reset-Method.
  • Services are private by default. This has to be adjusted mostly in tests.
  • ResolveDefinitionTemplatesPass is not available in v4.0, but ResolveChildDefinitionsPass is not available in 3.3 (tests only).

Closes #48
Closes #50

php7.2 runs against highest dependencies, php7.1 against lowest
Some things need to be adjusted to get it working for symfony 4.
 - DataCollectorInterface has a reset-Method.
 - Services are private by default. This has to be adjusted mostly in
   tests.
 - ResolveDefinitionTemplatesPass is not available in v4.0, but
   ResolveChildDefinitionsPass is not available in 3.3 (tests only).
It is explicitly added to phpunit later and would slow down phpcs.
@prolic
Copy link
Member

prolic commented Dec 7, 2017

@UFOMelkor I suggest we test PHP 7.2 with highest AND lowest dependencies and PHP 7.1 also with highest AND lowest dependencies. This will make 4 travis builds and will take a little longer, but we are more safe like this.

@UFOMelkor
Copy link
Member Author

Changed it to:

  • PHP 7.2 and 7.1 each with highest and lowest dependencies
  • coverage and phpcs only on PHP 7.2 with highest dependencies

class: ProophTest\Bundle\ServiceBus\DependencyInjection\Fixture\Model\AcmeUserEventListener
public: true
Copy link

Choose a reason for hiding this comment

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

Missing newline

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you, I added one 👍

@mumia
Copy link

mumia commented Dec 9, 2017

Any thoughts when this will be merged?

@UFOMelkor
Copy link
Member Author

ping @prooph/symfony-team

@codeliner
Copy link
Member

great job

@codeliner codeliner merged commit 91e0e8e into prooph:master Dec 13, 2017
@UFOMelkor UFOMelkor deleted the feature/php7.2-and-symfony4 branch December 14, 2017 06:41
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.

4 participants