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

Apply Doctrine CS v5 #855

Merged
merged 1 commit into from Oct 29, 2018
Merged

Apply Doctrine CS v5 #855

merged 1 commit into from Oct 29, 2018

Conversation

carusogabriel
Copy link
Contributor

No description provided.

@@ -63,7 +63,7 @@ jobs:
env: CODING_STANDARDS
php: 7.1
install:
- travis_retry composer require -n --prefer-dist --dev doctrine/coding-standard:^4.0
- travis_retry composer require -n --prefer-dist --dev doctrine/coding-standard:^5.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there some reason why we don't require it in the composer.json itself?

Copy link
Member

Choose a reason for hiding this comment

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

I think it is probably just old and can be updated to match other repos.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason is that DoctrineBundle sadly still supports dead PHP versions.

@Ocramius
Copy link
Member

@carusogabriel rebase, and then I'll just merge this 👍

@carusogabriel
Copy link
Contributor Author

@Ocramius Should be RTM 👍

@Ocramius
Copy link
Member

Hmm, still got PHP 5 on this thing, and it is failing. Can you please remove it, and bump the minimum version to 7.1?

Yes, it really is time.

@Majkl578
Copy link
Contributor

Can you please remove it, and bump the minimum version to 7.1?

@kimhemsoe planned to do that for 2.0.

@carusogabriel
Copy link
Contributor Author

@Ocramius I got a running build when I opened this PR: https://travis-ci.org/doctrine/DoctrineBundle/builds/433228829.

Maybe we should retry?

Can you please remove it, and bump the minimum version to 7.1?

See @Majkl578's #855 (comment), as well, I'd go with keeping this PR only related to Coding Standard changes :)

@Ocramius
Copy link
Member

Right, then 🚢

@Ocramius Ocramius merged commit 11539a0 into doctrine:master Oct 29, 2018
@carusogabriel carusogabriel deleted the doctrine-cs-v5 branch October 29, 2018 23:33
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;
use function method_exists;
Copy link
Member

Choose a reason for hiding this comment

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

This line breaks the Symfony CI, fixed in #869

@kimhemsoe kimhemsoe added this to the 1.9.2 milestone Oct 30, 2018
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.

None yet

6 participants