8000 [WIP] Require min. PHP 7.1 and Symfony 3.4 by TomasVotruba · Pull Request #81 · CoreSphere/ConsoleBundle · GitHub
[go: up one dir, main page]
Skip to content

[WIP] Require min. PHP 7.1 and Symfony 3.4 #81

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 17 commits into from
Closed

[WIP] Require min. PHP 7.1 and Symfony 3.4 #81

wants to merge 17 commits into from

Conversation

TomasVotruba
Copy link
Contributor
@TomasVotruba TomasVotruba commented Dec 11, 2017 •
edited
Loading

Related Changes

  • move code to /src and tests to /tests instead of mixing code with meta files in repository root
    • this allows to use static analysis with ease, do not exclude Tests or vendor manually for each one
    • and orientate in code, since this is PHP standard in open-source packages

@TomasVotruba TomasVotruba changed the title Require min. PHP 7.1 Require min. PHP 7.1 and Symfony 3.4 Dec 11, 2017
@TomasVotruba TomasVotruba changed the title Require min. PHP 7.1 and Symfony 3.4 [WIP] Require min. PHP 7.1 and Symfony 3.4 Dec 11, 2017
@TomasVotruba
Copy link
Contributor Author

I need help with tests. There is some custom command autloading, which is not possible since Symfony 3.4, resp. Symfony 4.

Is that external doctrine bundle needed for something or just random bundle?

I'd recommend adding those command in config manually instead.

# tests/services.yml
services:
    _defaults:
        autoconfigure: true

    SomeCommand: ~

@laszlokorte
Copy link
Member

According to git blame you added the doctrine dependency some time ago:
ffccee7

@TomasVotruba
Copy link
Contributor Author

Oh, shit on myself :D 🤕 🌴 thanks past me!

I'll take care of this

@TomasVotruba
Copy link
Contributor Author
TomasVotruba commented Dec 12, 2017 •
edited
Loading

Sources for Flex package upgrade:

@TomasVotruba
Copy link
Contributor Author

I'm sorry but It took me much more time than I expected and I don't have such great experience with imlementing packages under new Flex architecture, nor know how to test it easily. I'm not able to put more to this, but feel free to taky any part of this and use in your own PRs.

@laszlokorte
Copy link
Member

Thank you for your afford! I will see if I get some time to work on this but i seems that now only the someNonExistingCommand test is failing? Maybe somebody else can have a look as well? @core23 ?

@core23
Copy link
Contributor
core23 commented Feb 3, 2018

Sorry, not using this bundle anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TomasVotruba @laszlokorte @core23
0